> On May 22, 2013, 12:55 a.m., Stanton Sievers wrote: > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/conf/shindig.properties, > > lines 228-229 > > <https://reviews.apache.org/r/11299/diff/2/?file=295178#file295178line228> > > > > It is not clear to me why these are lists. The code that reads these > > seems to always take the first item in the list. Is there a reason to make > > this a comma-separated field? > > Dan Dumont wrote: > I agree. If this is a List, then it should keep trying to find a > supported algorithm until the list is exhausted. Otherwise, I think the > property would be better defined as a single value. > > Rich Thompson wrote: > The choice of list was driven by the need to enhance the OAuth support. > The OAuth spec says the the Provider specifies in its metadata what > algorithms (ordered list) are supported, but my quick read said that Shindig > just ignored this and always used SHA-1. This needs to be enhanced (separate > effort, not currently planned) to determine the first preferred consumer > algorithm that is in the Provider list. One reason not to push this too hard > now is the lack of complaints about the behavior ... pretty clear all > Providers people have tried support SHA-1. > > Ryan Baxter wrote: > Rich the algorithm used is specified in the oauth.json file in shindig. > I know SmartCloud only supports plain text, I ran into that a while ago. > > Zhi Hong Yang wrote: > the setting is telling user what sha algorithms can be supported. and the > first one is the sha algorithms in using. > > Ryan Baxter wrote: > Still dont understand the need for a list here is we only ever use the > first one. Could someone elaborate? > > Zhi Hong Yang wrote: > Since the list is no use for now and may confuse the user, I agree just > just the value instead list, All agreee?
It would be fine for this to drop from a list to a singular value (all that is need for the internal use cases updated by this patch), but I still think there is significant value in providing automatic matching when dealing with external services (such as OAuth) rather than depending on an administrator entry of the overlap in what is supported on both the local deployment and the remote system and that dropping to a singular value is removing the foundation for that work (i.e. would have to be added back later). - Rich ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11299/#review20878 ----------------------------------------------------------- On July 18, 2013, 5:41 a.m., Zhi Hong Yang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/11299/ > ----------------------------------------------------------- > > (Updated July 18, 2013, 5:41 a.m.) > > > Review request for shindig, Dan Dumont, Ryan Baxter, Rich Thompson, and > Stanton Sievers. > > > Repository: shindig > > > Description > ------- > > > the following setting are added to support different algorithms: > > 1) shindig.crypo.preferredHashAlgorithms = SHA, SHA-256, SHA-384, SHA-512 > > this setting is used to set string hash algorithm, all supported hash > Algorithms is listed and the first one is supported by default. > > 2) shindig.crypo.preferredHMACAlgorithms = > HMACSHA1,HMACSHA256,HMACSHA384,HMACSHA512 > > this setting is used to set string encrypt/decrypt algorithm, all supported > HMA SHA algorithms is listed and the first one is supported by default > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodec.java > 1503103 > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/crypto/BasicBlobCrypter.java > 1503103 > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/crypto/Crypto.java > 1503103 > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/util/DigestType.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/util/GenericDigestUtils.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/util/HMACType.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodecTest.java > 1503103 > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/BlobCrypterSecurityTokenTest.java > 1503103 > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/common/crypto/BlobCrypterTest.java > 1503103 > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/common/crypto/CryptoTest.java > 1503103 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGuiceModule.java > 1503103 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/HashLockedDomainService.java > 1503103 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthRequest.java > 1503103 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/MacTokenHandler.java > 1503103 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/HashShaLockedDomainPrefixGenerator.java > 1503103 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/testing/FakeOAuthServiceProvider.java > 1503103 > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth/OAuthAuthenticationHandler.java > 1503103 > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/FakeOAuthRequest.java > 1503103 > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth/OAuthAuthenticationHanderTest.java > 1503103 > > Diff: https://reviews.apache.org/r/11299/diff/ > > > Testing > ------- > > Done. > > > Thanks, > > Zhi Hong Yang > >
