I agree (see my previous email) that not returning UnsupportedOperationException is a bad idea. I do not agree that using null to represent an anon token is a good idea.
In the social code (and the social code only - as this is where the anon token was originally used and designed for), we use null to mean that a person could not authenticate properly and the anon token means that they did authenticate properly - just anonymously. In this way some containers can choose to support anon tokens and others can choose not to. In the social code the interface impls are the ones expected to check whether isAnon is true. So, the containers that do support anon tokens have to deal with it - those who don't support them can ignore the fact that they exist but both sets of contaiers get to benefit from common code that says if(securityToken == null) return the proper restful error code for not being authenticated. Perhaps this got integrated into the rendering code improperly (and i think it did - because that code doesn't respect the guice binding for whether or not anon tokens are even allowed) but I don't think this should be changed for the social code. - Cassie On Fri, Sep 5, 2008 at 2:49 PM, Brian Eaton <[EMAIL PROTECTED]> wrote: > [+shindig-dev, with John's permission] > > Please don't have an anonymous security token return real values for > any of the fields that identify a user or a gadget. Having watched > this play out, I have a strong preference for removing the anonymous > token entirely. > > As I recall, the original rationale for the token was that it was > easier to check "securityToken.isAnonymous()" than "securityToken != > null". > > Then someone (I haven't checked who) submitted code to Shindig that > crashed all unauthenticated gadget renders because they did not change > existing code to check securityToken.isAnonymous() instead of > securityToken != null. > > In order to fix that, we're now talking about making an anonymous > security token that acts and behaves almost exactly like a real > security token. If someone forgets to check "isAnonymous", they are > creating a security problem. For example, as far as the signed fetch > and OAuth code are concerned "" is a completely legitimate user id. > > This entire direction seems like a mistake, and a poorly tested one at > that. "AnymousSecurityToken" is a backwards idea. We should go back > to using null. > > Cheers, > Brian > > On Fri, Sep 5, 2008 at 2:32 PM, John Hjelmstad <[EMAIL PROTECTED]> wrote: > > FYI -- I was interested in both your opinion on whether this change is > the > > right handling of this situation, or if perhaps null should instead be > > returned for Strings. I wanted to err on the side of usage safety to > avoid > > NPEs but don't want to bump into weird side effects if nulls are actually > > expected. > > Thx, > > John > > > > ---------- Forwarded message ---------- > > From: <[EMAIL PROTECTED]> > > Date: Fri, Sep 5, 2008 at 2:28 PM > > Subject: svn commit: r692557 - > > > /incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/AnonymousSecurityToken.java > > To: [EMAIL PROTECTED] > > > > > > Author: johnh > > Date: Fri Sep 5 14:28:17 2008 > > New Revision: 692557 > > > > URL: http://svn.apache.org/viewvc?rev=692557&view=rev > > Log: > > Returning safe default values from AnonymousSecurityToken rather than > > throwing UnsupportedOperationException. This prevents Shindig run in Full > > mode (which has UrlAuthHandler and AnonymousAuthHandler configured) not > to > > barf when direct-rendering a gadget (/gadgets/ifr?url=foo.xml) without a > > security token. > > > > > > Modified: > > > > > > incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/AnonymousSecurityToken.java > > > > Modified: > > > incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/AnonymousSecurityToken.java > > URL: > > > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/AnonymousSecurityToken.java?rev=692557&r1=692556&r2=692557&view=diff > > > ============================================================================== > > --- > > > incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/AnonymousSecurityToken.java > > (original) > > +++ > > > incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/AnonymousSecurityToken.java > > Fri Sep 5 14:28:17 2008 > > @@ -32,38 +32,38 @@ > > } > > > > public String toSerialForm() { > > - throw new UnsupportedOperationException(); > > + return ""; > > } > > > > public String getOwnerId() { > > - throw new UnsupportedOperationException(); > > + return ""; > > } > > > > public String getViewerId() { > > - throw new UnsupportedOperationException(); > > + return ""; > > } > > > > public String getAppId() { > > - throw new UnsupportedOperationException(); > > + return ""; > > } > > > > public String getDomain() { > > - throw new UnsupportedOperationException(); > > + return ""; > > } > > > > public String getAppUrl() { > > - throw new UnsupportedOperationException(); > > + return ""; > > } > > > > public long getModuleId() { > > - throw new UnsupportedOperationException(); > > + return 0L; > > } > > > > public String getUpdatedToken() { > > - throw new UnsupportedOperationException(); > > + return ""; > > } > > > > public String getTrustedJson() { > > - throw new UnsupportedOperationException(); > > + return ""; > > } > > } > > \ No newline at end of file > > > > > > > > >
