On Sat, May 8, 2010 at 7:33 AM, Paul Lindner <[email protected]> wrote:
> I'd like to add isExpired() to the SecurityToken API to support use-cases
> where the expiration time is not known but the token is known to be
> expired.
> To avoid boilerplate I've created an AbstractSecurityToken class that
> implementations can extend. It has this method:
>
> boolean isExpired() {
> Long expiresAt = getExpiresAt();
> return (expiresAt == null) ? false : System.currentTimeMillis() >
> expiresAt;
> }
>
An abstract base class helps avoid boilerplate, but I'm curious what use
cases you have in mind for tokens that are
expired-but-not-because-expiry-time-has-passed.
The one that comes to mind for me are single-use tokens, which might have a
hybrid scheme (for example): expires by time X *OR* if ever used at all,
whichever comes first.
In such case, an isExpired method definitely makes good sense, though I'd
remain very mildly concerned about code paths that confuse the interface's
API use ie. checking getExpiredAt() when isExpired() says "false" anyway. I
guess the best we can do in this situation is just document that isExpired()
takes primacy over getExpiredAt()? Or do you have any other thoughts on
specific use cases?
>
> Also, more questions
>
> * getDomain() is only called once, it's used to add a default consumer key
> for oauth requests. Since this is set by the ContainerConfig might it make
> sense to not pass around this info in the token and instead grab it from
> the
> config as needed? We already store the container name in the token.
>
To be concrete, the idea is to drop getDomain() from the API and replace it
w/ something ContainerConfig driven?
Seems sensible, though I'd defer to Brian Eaton and others for input. The
only vague concern is that ContainerConfig values can generally be spoofed,
since container values can trivially be switched out. I'm not sure if
there's any issue there.
>
> * BlobCrypter classes use a fixed expiration time and stores the generated
> time instead. Refactoring will be needed here...
>
Agreed.
Cheers,
John
>
> On Fri, May 7, 2010 at 7:55 PM, John Hjelmstad <[email protected]> wrote:
>
> > On Fri, May 7, 2010 at 5:59 PM, Paul Lindner <[email protected]> wrote:
> >
> > > I propose that we add expiration methods to the SecurityToken
> interface.
> > > This is necessary for future OAuth2 compatibility. Here's the
> proposed
> > > interface:
> > >
> > > /**
> > > * @return the UTC timestamp (in milliseconds) that this token expires
> > or
> > > null if unknown or indeterminate
> > > */
> > > Long getExpiresAt();
> > >
> >
> > +1
> >
> >
> > >
> > > /**
> > > * @return true if the token is no longer valid
> > > */
> > > boolean isExpired();
> > >
> >
> > +0 or maybe -0.1 -- is this really necessary? Seems like boilerplate.
> >
> > --j
> >
>
