The offending line of code is GadgetRenderingTask.java:448:
String updatedToken = authToken.getUpdatedToken();
I'm happy to change that code to do the right thing when isAnonymous() --
but I'm interested to hear the implications of returning "safe" dummy values
as opposed to throwing a runtime exception for each operation.
As Brian says, it seems a little weird to pretend the anonymous security
token is a valid one, which does suggest that UnsupportedOperationException
is reasonable, since it's essentially a programming error to call anything
on a security token if (token.isAnonymous()). That contract for
SecurityToken seems a little funky to me, but I don't have a strong opinion
right now on it.
--John
On Fri, Sep 5, 2008 at 2:55 PM, Cassie <[EMAIL PROTECTED]> wrote:
> 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
>> >
>> >
>> >
>> >
>>
>
>
