After the token manager was made stateless the full code is sent in emails (reset password and verify email), this is not very nice as it's very long. Two ideas on how to fix this: 1. Save the code (user sessions?) and convert back to sending just the code id in the email 2. Send the info required to create a code (clientId, scope, state and redirect encoded with the realm key) 3. Send a short code that has to be copied/pasted back into the current login form My thoughts are: 1. Nice and simple, but requires "storing" the code temporarily. Another thing we could do is to associate it with the session, this would make sure the email can only be clicked by the user that actually initiated it. 2. Not so nice as I think it'll still create too long links (especially if redirect and state are big). 3. Kinda nice, but changes the way it all works. This may actually be the optimal and more secure way to do it though. See https://issues.jboss.org/browse/KEYCLOAK-542 for how big the link in the email actually is ;) _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/keycloak-dev

> If that's what you're saying +1. Are you referring to option 1, storing the required info in the user session temporarily? Not sure I understand the details about what you're proposing though.

Are you talking about reducing the size of the code altogether or just for the email links? I was thinking about just saving the base64 encoded access-code with the user session temporarily, then sending the the access code id (36 char uuid as before) in the email. First time the user clicks on the link the access code would be removed from the session, so this would also make the links a one-time-click thing. ----- Original Message ----- > From: "Bill Burke" <bburke(a)redhat.com&gt; > To: "Stian Thorgersen" <stian(a)redhat.com&gt; > Cc: keycloak-dev(a)lists.jboss.org > Sent: Tuesday, 15 July, 2014 6:14:45 PM > Subject: Re: [keycloak-dev] Reset password and verify email links are to long > > > > On 7/15/2014 12:34 PM, Stian Thorgersen wrote: >>> If that's what you're saying +1. >> Are you referring to option 1, storing the required info in the user >> session temporarily? Not sure I understand the details about what you're >> proposing though. >> > Yes, option 1. AccessCode should be associated with the user session. > Appropriate state needs to be stored in the session as AccessCode > currently saves a lot of stuff. > > Then the code only has to contain: > > id, session-id, timestamp > > > > -- > Bill Burke > JBoss, a division of Red Hat > http://bill.burkecentral.com > _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/keycloak-dev

This is probably what you've said already Bill, but just to make sure: 1. Associate the required information to create a token from an access code with the user session (basically what's in AccessCodeEntry now) 2. The code that is sent as the query param only contains id, session-id, timestamp 3. Once we receive a code to swap for a token we remove the information added in 1 from the user session and use this to generate the token Couple questions: * Do we do this just for emails? or also for the code sent in login redirects?

* Do we really need session-id and timestamp, or isn't id enough?

* Isn't this pretty much just going back to state-full TokenManager except we're saving it in the UserSession instead of TokenManager itself?

----- Original Message ----- > From: "Marek Posolda" <mposolda(a)redhat.com&gt; > To: "Stian Thorgersen" <stian(a)redhat.com&gt;, "Bill Burke" <bburke(a)redhat.com&gt; > Cc: keycloak-dev(a)lists.jboss.org > Sent: Wednesday, 16 July, 2014 11:34:39 AM > Subject: Re: [keycloak-dev] Reset password and verify email links are to long > > +1 to associate code with the UserSession and remove it once code is > exchanged. This will also help to fix the issue we discussed before, > that now it's possible to exchange same code multiple times, which is > not in line with OAuth2 specs . Created > https://issues.jboss.org/browse/KEYCLOAK-560 and linked with > https://issues.jboss.org/browse/KEYCLOAK-542 > > Marek > > On 16.7.2014 10:58, Stian Thorgersen wrote: >> Are you talking about reducing the size of the code altogether or just for >> the email links? >> >> I was thinking about just saving the base64 encoded access-code with the >> user session temporarily, then sending the the access code id (36 char >> uuid as before) in the email. First time the user clicks on the link the >> access code would be removed from the session, so this would also make the >> links a one-time-click thing. >> >> ----- Original Message ----- >>> From: "Bill Burke" <bburke(a)redhat.com&gt; >>> To: "Stian Thorgersen" <stian(a)redhat.com&gt; >>> Cc: keycloak-dev(a)lists.jboss.org >>> Sent: Tuesday, 15 July, 2014 6:14:45 PM >>> Subject: Re: [keycloak-dev] Reset password and verify email links are to >>> long >>> >>> >>> >>> On 7/15/2014 12:34 PM, Stian Thorgersen wrote: >>>>> If that's what you're saying +1. >>>> Are you referring to option 1, storing the required info in the user >>>> session temporarily? Not sure I understand the details about what you're >>>> proposing though. >>>> >>> Yes, option 1. AccessCode should be associated with the user session. >>> Appropriate state needs to be stored in the session as AccessCode >>> currently saves a lot of stuff. >>> >>> Then the code only has to contain: >>> >>> id, session-id, timestamp >>> >>> >>> >>> -- >>> Bill Burke >>> JBoss, a division of Red Hat >>> http://bill.burkecentral.com >>> >> _______________________________________________ >> keycloak-dev mailing list >> keycloak-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/keycloak-dev >

This is probably what you've said already Bill, but just to make sure: 1. Associate the required information to create a token from an access code with the user session (basically what's in AccessCodeEntry now) 2. The code that is sent as the query param only contains id, session-id, timestamp 3. Once we receive a code to swap for a token we remove the information added in 1 from the user session and use this to generate the token Couple questions: * Do we do this just for emails? or also for the code sent in login redirects? * Do we really need session-id and timestamp, or isn't id enough?

* Isn't this pretty much just going back to state-full TokenManager except we're saving it in the UserSession instead of TokenManager itself?

From: "Bill Burke" <bburke(a)redhat.com&gt; To: keycloak-dev(a)lists.jboss.org Sent: Wednesday, 16 July, 2014 1:47:53 PM Subject: Re: [keycloak-dev] Reset password and verify email links are to long On 7/16/2014 8:43 AM, Bill Burke wrote: > > > On 7/16/2014 6:54 AM, Stian Thorgersen wrote: >> This is probably what you've said already Bill, but just to make sure: >> >> 1. Associate the required information to create a token from an access >> code with the user session (basically what's in AccessCodeEntry now) >> 2. The code that is sent as the query param only contains id, session-id, >> timestamp >> 3. Once we receive a code to swap for a token we remove the information >> added in 1 from the user session and use this to generate the token >> >> Couple questions: >> >> * Do we do this just for emails? or also for the code sent in login >> redirects? >> * Do we really need session-id and timestamp, or isn't id enough? > > Actually, do we even need a specific access code? Even for OAuth 2 > flow? Just pass around the session id. All information to validate > calls, especially accessCodeToToken[1] should be in the UserSession. > You just have to make absolutely sure you are validating redirect uri > and client-id to guard against swapping. > Actually maybe the access code should be a digitally signed session-id? Then you're fully protected from people guessing session-ids. Granted, the window to guess is relatively short. *shrug* I don't know. :)

>> * Isn't this pretty much just going back to state-full TokenManager except >> we're saving it in the UserSession instead of TokenManager itself? >> > > Yup. :) LOL!!!!! > > [1] http://tools.ietf.org/html/rfc6749#section-4.1.3 > > > -- Bill Burke JBoss, a division of Red Hat http://bill.burkecentral.com _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/keycloak-dev

From: "Marek Posolda" <mposolda(a)redhat.com&gt; To: "Stian Thorgersen" <stian(a)redhat.com&gt;, "Bill Burke" <bburke(a)redhat.com&gt; Cc: keycloak-dev(a)lists.jboss.org Sent: Wednesday, 16 July, 2014 2:05:15 PM Subject: Re: [keycloak-dev] Reset password and verify email links are to long On 16.7.2014 14:56, Stian Thorgersen wrote: > > ----- Original Message ----- >> From: "Bill Burke" <bburke(a)redhat.com&gt; >> To: keycloak-dev(a)lists.jboss.org >> Sent: Wednesday, 16 July, 2014 1:47:53 PM >> Subject: Re: [keycloak-dev] Reset password and verify email links are to >> long >> >> >> >> On 7/16/2014 8:43 AM, Bill Burke wrote: >>> >>> On 7/16/2014 6:54 AM, Stian Thorgersen wrote: >>>> This is probably what you've said already Bill, but just to make sure: >>>> >>>> 1. Associate the required information to create a token from an access >>>> code with the user session (basically what's in AccessCodeEntry now) >>>> 2. The code that is sent as the query param only contains id, >>>> session-id, >>>> timestamp >>>> 3. Once we receive a code to swap for a token we remove the information >>>> added in 1 from the user session and use this to generate the token >>>> >>>> Couple questions: >>>> >>>> * Do we do this just for emails? or also for the code sent in login >>>> redirects? >>>> * Do we really need session-id and timestamp, or isn't id enough? >>> Actually, do we even need a specific access code? Even for OAuth 2 >>> flow? Just pass around the session id. All information to validate >>> calls, especially accessCodeToToken[1] should be in the UserSession. >>> You just have to make absolutely sure you are validating redirect uri >>> and client-id to guard against swapping. >>> >> Actually maybe the access code should be a digitally signed session-id? >> Then you're fully protected from people guessing session-ids. >> Granted, the window to guess is relatively short. *shrug* I don't know. >> :) > It can't just be session-id as there's multiple apps/clients per-session, > also even multiple logins for a single app. How about session-id + > client-id + timestamp, and we sign it as well? Sorry to mention it again, but looks that this is still not enough to help with the requirement of "Authorization codes MUST be short lived and single-use." of http://tools.ietf.org/html/rfc6749#section-10.5 :-)

Marek > >>>> * Isn't this pretty much just going back to state-full TokenManager >>>> except >>>> we're saving it in the UserSession instead of TokenManager itself? >>>> >>> Yup. :) LOL!!!!! >>> >>> [1] http://tools.ietf.org/html/rfc6749#section-4.1.3 >>> >>> >>> >> -- >> Bill Burke >> JBoss, a division of Red Hat >> http://bill.burkecentral.com >> _______________________________________________ >> keycloak-dev mailing list >> keycloak-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/keycloak-dev >> > _______________________________________________ > keycloak-dev mailing list > keycloak-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/keycloak-dev

From: "Bill Burke" <bburke(a)redhat.com&gt; To: "Stian Thorgersen" <stian(a)redhat.com&gt;, "Marek Posolda" <mposolda(a)redhat.com&gt; Cc: keycloak-dev(a)lists.jboss.org Sent: Wednesday, 16 July, 2014 2:14:45 PM Subject: Re: [keycloak-dev] Reset password and verify email links are to long On 7/16/2014 9:09 AM, Stian Thorgersen wrote: > > > ----- Original Message ----- >> From: "Marek Posolda" <mposolda(a)redhat.com&gt; >> To: "Stian Thorgersen" <stian(a)redhat.com&gt;, "Bill Burke" >> <bburke(a)redhat.com&gt; >> Cc: keycloak-dev(a)lists.jboss.org >> Sent: Wednesday, 16 July, 2014 2:05:15 PM >> Subject: Re: [keycloak-dev] Reset password and verify email links are to >> long >> >> On 16.7.2014 14:56, Stian Thorgersen wrote: >>> >>> ----- Original Message ----- >>>> From: "Bill Burke" <bburke(a)redhat.com&gt; >>>> To: keycloak-dev(a)lists.jboss.org >>>> Sent: Wednesday, 16 July, 2014 1:47:53 PM >>>> Subject: Re: [keycloak-dev] Reset password and verify email links are to >>>> long >>>> >>>> >>>> >>>> On 7/16/2014 8:43 AM, Bill Burke wrote: >>>>> >>>>> On 7/16/2014 6:54 AM, Stian Thorgersen wrote: >>>>>> This is probably what you've said already Bill, but just to make sure: >>>>>> >>>>>> 1. Associate the required information to create a token from an access >>>>>> code with the user session (basically what's in AccessCodeEntry now) >>>>>> 2. The code that is sent as the query param only contains id, >>>>>> session-id, >>>>>> timestamp >>>>>> 3. Once we receive a code to swap for a token we remove the >>>>>> information >>>>>> added in 1 from the user session and use this to generate the token >>>>>> >>>>>> Couple questions: >>>>>> >>>>>> * Do we do this just for emails? or also for the code sent in login >>>>>> redirects? >>>>>> * Do we really need session-id and timestamp, or isn't id enough? >>>>> Actually, do we even need a specific access code? Even for OAuth 2 >>>>> flow? Just pass around the session id. All information to validate >>>>> calls, especially accessCodeToToken[1] should be in the UserSession. >>>>> You just have to make absolutely sure you are validating redirect uri >>>>> and client-id to guard against swapping. >>>>> >>>> Actually maybe the access code should be a digitally signed session-id? >>>> Then you're fully protected from people guessing session-ids. >>>> Granted, the window to guess is relatively short. *shrug* I don't know. >>>> :) >>> It can't just be session-id as there's multiple apps/clients per-session, >>> also even multiple logins for a single app. How about session-id + >>> client-id + timestamp, and we sign it as well? >> Sorry to mention it again, but looks that this is still not enough to >> help with the requirement of "Authorization codes MUST be short lived >> and single-use." of http://tools.ietf.org/html/rfc6749#section-10.5 :-) > > Short lived would be covered as there's a timestamp. For single-use we'd > have to add the timestamp to the user session or somethin' > See my last email. Just store everything within ClientUserSessionAssociationEntity and have a ClientUserSessionAssociationEntity per access-code. class ClientUserSessionAssociationEntity { @Id String accessCode; @ManyToOne UserSessionEntity session; boolean accessCodeActive; long timestamp; String clientId; } -- Bill Burke JBoss, a division of Red Hat http://bill.burkecentral.com

From: "Marek Posolda" <mposolda(a)redhat.com&gt; To: "Stian Thorgersen" <stian(a)redhat.com&gt;, "Bill Burke" <bburke(a)redhat.com&gt; Cc: keycloak-dev(a)lists.jboss.org Sent: Thursday, 17 July, 2014 6:09:54 PM Subject: Re: [keycloak-dev] Reset password and verify email links are to long On 17.7.2014 17:00, Stian Thorgersen wrote: > Just started on this. Quick summary of my attack plan: > > 1. Remove AccessToken from AccessCode. Instead add List<String> > rolesRequested and create AccessToken in TokenService.accessCodeToToken. > Any reasons why this wouldn't work? > 2. Move some info from AccessCode to UserSession, and remove anything > redundant. For example username used to login, auth-method (social or > form), remember-me. > 3. Add info from AccessCode to UserSessionProvider and send signed key > instead as the code query param. > > With regards to the required info here's what I've come up with: > > To verify the code: > > * Need to know it hasn't been done before - can either be done by having a > timestamp on ClientAssociation that is incremented every time a code is > requested/used, or by having a ClientAssociation per-code. As we'll need > to store more specific to a code than just the fact it has been used or > not, I think it's best to just have a ClientAssociation per-code > * Not expired - no prob > * Session active - no prob > * redirect_uri when retrieving code matches redirect_uri query param when > swapping for token - we don't do this currently I think, but spec requires > it. This would require a ClientAssociation per-code. > * Correct client_id - no prob, just make sure code belongs to the client_id > query param > > Then to create the token we need: > > * List of roles requested - the union of user roles and app/client scope. > This would require a ClientAssociation per-code as the role-mappings/scope > could change between user granted the roles to the client and the code is > exchanged. Also in the future once we add a scope query param this will be > required. Did you mean intersection rather than union? > * Realm - no prob, user-session is associated with a realm > * Client - no prob, ClientAssociation is associated with a client > * User - no prob, user-session is associated with a user > * Session - no prob, user-session is a session;) > > All in all it seems like a bit more work than I initially considered. Or am > I attacking this completely wrong?!? To me it makes sense and should work. I wonder about a bit less-time consuming flow to implement, when AccessCode is generated together with AccessToken after authentication as it's now (in OAuthFlows.processAccessCode) . Only new thing in OAuthFlows.processAccessCode will be that ClientAssociation with just key-value pair of <signed-codeId>:<base64-encoded-AccessCode> will be associated with UserSession. Signed codeId will be then saved as "code" parameter . Then once client will want to exchange code, Keycloak will perform lookup of correct UserSession based on codeId (new method on UserSessionProvider like: getSessionbyCode). Then when UserSession is successfully looked-up, ClientAssociation is immediatelly deleted, so that it's not possible to exchange same code twice. In TokenService.accessCodeToToken, we will still have AccessCode with wrapped AccessToken so we can do all verification is it's done now. Only new thing might be redirect_uri verification, which is not currently done. Defacto it will be same as current flow, only new thing will be: - ClientAssociation with the pair <signed-codeId>:<base64-encoded-AccessCode>, created when code is generated and deleted when code is exchanged. - redirect_uri validation during exchange code to token. I think that this approach will help us to address 2 main issues, which we currently have (long "code" parameter and same code is possible to exchange more times). Only thing is, that base64-encoded-AccessCode will be quite long and will contain redundant info, but is it big problem? If we want later add new info to AccessCode, we don't need to do any changes in UserSessionProvider model as it will be still just base64 encoded string. Not sure which approach is better, just a though:-)

Marek

On 16.7.2014 14:56, Stian Thorgersen wrote: > ----- Original Message ----- >> From: "Bill Burke" <bburke(a)redhat.com&gt; >> To: keycloak-dev(a)lists.jboss.org >> Sent: Wednesday, 16 July, 2014 1:47:53 PM >> Subject: Re: [keycloak-dev] Reset password and verify email links are to long >> >> >> >> On 7/16/2014 8:43 AM, Bill Burke wrote: >>> On 7/16/2014 6:54 AM, Stian Thorgersen wrote: >>>> This is probably what you've said already Bill, but just to make sure: >>>> >>>> 1. Associate the required information to create a token from an access >>>> code with the user session (basically what's in AccessCodeEntry now) >>>> 2. The code that is sent as the query param only contains id, session-id, >>>> timestamp >>>> 3. Once we receive a code to swap for a token we remove the information >>>> added in 1 from the user session and use this to generate the token >>>> >>>> Couple questions: >>>> >>>> * Do we do this just for emails? or also for the code sent in login >>>> redirects? >>>> * Do we really need session-id and timestamp, or isn't id enough? >>> Actually, do we even need a specific access code? Even for OAuth 2 >>> flow? Just pass around the session id. All information to validate >>> calls, especially accessCodeToToken[1] should be in the UserSession. >>> You just have to make absolutely sure you are validating redirect uri >>> and client-id to guard against swapping. >>> >> Actually maybe the access code should be a digitally signed session-id? >> Then you're fully protected from people guessing session-ids. >> Granted, the window to guess is relatively short. *shrug* I don't know. :) > It can't just be session-id as there's multiple apps/clients per-session, also even multiple logins for a single app. How about session-id + client-id + timestamp, and we sign it as well? Sorry to mention it again, but looks that this is still not enough to help with the requirement of "Authorization codes MUST be short lived and single-use." of http://tools.ietf.org/html/rfc6749#section-10.5 :-)

Marek >>>> * Isn't this pretty much just going back to state-full TokenManager except >>>> we're saving it in the UserSession instead of TokenManager itself? >>>> >>> Yup. :) LOL!!!!! >>> >>> [1] http://tools.ietf.org/html/rfc6749#section-4.1.3 >>> >>> >>> >> -- >> Bill Burke >> JBoss, a division of Red Hat >> http://bill.burkecentral.com >> _______________________________________________ >> keycloak-dev mailing list >> keycloak-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/keycloak-dev >> > _______________________________________________ > keycloak-dev mailing list > keycloak-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/keycloak-dev _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/keycloak-dev

From: "Bill Burke" <bburke(a)redhat.com&gt; To: "Stian Thorgersen" <stian(a)redhat.com&gt; Cc: keycloak-dev(a)lists.jboss.org Sent: Wednesday, 16 July, 2014 2:09:22 PM Subject: Re: [keycloak-dev] Reset password and verify email links are to long On 7/16/2014 8:56 AM, Stian Thorgersen wrote: > > > ----- Original Message ----- >> From: "Bill Burke" <bburke(a)redhat.com&gt; >> To: keycloak-dev(a)lists.jboss.org >> Sent: Wednesday, 16 July, 2014 1:47:53 PM >> Subject: Re: [keycloak-dev] Reset password and verify email links are to >> long >> >> >> >> On 7/16/2014 8:43 AM, Bill Burke wrote: >>> >>> >>> On 7/16/2014 6:54 AM, Stian Thorgersen wrote: >>>> This is probably what you've said already Bill, but just to make sure: >>>> >>>> 1. Associate the required information to create a token from an access >>>> code with the user session (basically what's in AccessCodeEntry now) >>>> 2. The code that is sent as the query param only contains id, >>>> session-id, >>>> timestamp >>>> 3. Once we receive a code to swap for a token we remove the information >>>> added in 1 from the user session and use this to generate the token >>>> >>>> Couple questions: >>>> >>>> * Do we do this just for emails? or also for the code sent in login >>>> redirects? >>>> * Do we really need session-id and timestamp, or isn't id enough? >>> >>> Actually, do we even need a specific access code? Even for OAuth 2 >>> flow? Just pass around the session id. All information to validate >>> calls, especially accessCodeToToken[1] should be in the UserSession. >>> You just have to make absolutely sure you are validating redirect uri >>> and client-id to guard against swapping. >>> >> >> Actually maybe the access code should be a digitally signed session-id? >> Then you're fully protected from people guessing session-ids. >> Granted, the window to guess is relatively short. *shrug* I don't know. >> :) > > It can't just be session-id as there's multiple apps/clients per-session, > also even multiple logins for a single app. How about session-id + > client-id + timestamp, and we sign it as well? > OH yeah...right. Ugh. Dont you really just need access-code-id and session-id? UserSession stores the rest of the state needed to verify and expire codes, specific to each access-code. Maybe ClientUserSessionAssociationEntity transforms into this access-code storage. Then you could keep track of when a token was created for this client, when access code was created, state of access-code flow, etc.

-- Bill Burke JBoss, a division of Red Hat http://bill.burkecentral.com