Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verify email and accept invitation endpoint wouldn't redirect to callback with valid Authorization code #300

Closed
rsoletob opened this issue Feb 3, 2016 · 13 comments

Comments

@rsoletob
Copy link
Contributor

rsoletob commented Feb 3, 2016

Problem is explained in #229

This is the information inside token in verify email sent to email:

{
  "aud": "9jCU4aaDHjV-y59SSlGwfrmpdo4mIkGBW4E41QvI-X0=@127.0.0.1",
  "exp": 1454528184,
  "http://coreos.com/email/verification-callback": "http://127.0.0.1:5555/callback",
  "http://coreos.com/email/verificationEmail": "user@email.com",
  "iat": 1454484984,
  "iss": "http://127.0.0.1:5556",
  "sub": "b85b41f9-87bc-4355-a39f-1f5d740d50b7"
}

This is the information inside token in accept invitation sent to email:

{
  "aud": "9jCU4aaDHjV-y59SSlGwfrmpdo4mIkGBW4E41QvI-X0=@127.0.0.1",
  "exp": 1454525704,
  "http://coreos.com/email/verificationEmail": "user2@email.com",
  "http://coreos.com/invitation/callback": "http://127.0.0.1:5555/callback",
  "http://coreos.com/password/old-hash": "yZpQehIdksSz93TjfrSclrJdiBa2aOu4_w1isz0ccJs=",
  "iat": 1454482504,
  "iss": "http://127.0.0.1:5556",
  "sub": "d7fcf0ce-8c30-402d-8725-06c1f87d769a"
}

DEX can't create a valid code to exchange an Access token because it doesn't have all needed parameters. If you want to create a new session and its associated Authorization code you need some data like state, nonce and scopes. Theese parameters are set by the client and could be different in each registered client in DEX. (The issue #1 might be which take care of state parameter to prevent csrf, but in this issue DEX is intended to insert it in session to create an Authorization code). So DEX can't generate missing parameters and sent them inside token. Therefore DEX redirects End-User to callback URI without valid Authorization code that client can't process. Definition of state and nonce parameters according to OIDC spec:

-nonce: String value used to associate a Client session with an ID Token, and to mitigate replay attacks. The value is passed through unmodified from the Authentication Request to the ID Token. If present in the ID Token, Clients MUST verify that the nonce Claim Value is equal to the value of the nonce parameter sent in the Authentication Request. If present in the Authentication Request, Authorization Servers MUST include a nonce Claim in the ID Token with the Claim Value being the nonce value sent in the Authentication Request. Authorization Servers SHOULD perform no other processing on nonce values used. The nonce value is a case sensitive string.

-state: OAuth 2.0 state value. REQUIRED if the state parameter is present in the Authorization Request. Clients MUST verify that the state value is equal to the value of state parameter in the Authorization Request.

For this reason, I propose a new redirection URI to redirect a End-User when it comes from a email link (from a verify mail or invitation mail). This URI would be a main page for the client, for example in example-app, http://127.0.0.1/. At this point, a End-User goes to login link, uses the Authorization code flow and client receives the valid code to exchange an Access token. This approach solves problems explained in #229.

@ericchiang
Copy link
Contributor

Are you basically saying that the verify and accept email JWTs are missing a nonce and state claim?

@rsoletob
Copy link
Contributor Author

rsoletob commented Feb 4, 2016

Yes, that's right, but it isn't the gist, the main problem is DEX can't generate this values by itself.

@adrianlop
Copy link
Contributor

by the way, are you willing to use Gitter chat to discuss these matters instead of opening big issues first?
We saw you have IRC but we think it's "too big" since it targets every CoreOS project, right?

@ericchiang
Copy link
Contributor

@rsoletob Both of the JWTs you're discussing are outside of the scope of Open ID Connect. I don't know if either of these would require nonces. Because 1) the email verification should only work once anyway 2) the "http://coreos.com/password/old-hash" should also require changing your password to only work once.

As for your other concert, yes, it may not be possible to redirect with a valid redirect URL back to the client (particularly if the end-user has never interacted with the client). Maybe the registration API could take a nonce and state parameter to allow dex to create the correct redirect?

@adrianlop want to apologize about the current communications being sparse. I believe time zones aren't helping us here. We're looking to establish a good place for conversations around dex (google group, community hangout, etc.).

For now you can usually find me (or someone who would be willing to grab someone on the dex team) in #coreos on freenode.

@adrianlop
Copy link
Contributor

@ericchiang great, thanks!

@rsoletob
Copy link
Contributor Author

As I understand it, there are two approachs:

On the one hand, dex server couldn't give a valid authorization code because client needs some interaction with dex and end-user to generate state and nonce value. So one approach is redirect user to somewhere in order to client can request a valid Authorization code.

On the other hand, dex server could get these values using aditional info inside API's calls to create a new session. So the other approach is to somehow retrieve these values. The problem is if the client use these values to manage user's sessions. However if client can't work without these values it could request a new access token.

In my opinion, I prefer first approach because register only occurs one time and it isn't so annoying for the end-user insert its credential twice.

Maybe this would be discussed in other side (dev-mail for example) and this issue would be closed until there will be some conclusions about this. Do you agree?

@bobbyrullo
Copy link
Contributor

I'm happy to continue the discussion here - that way the history is all in one place for the curious. Of course, for a more interactive chat we can do IRC or something.

Anyhow you say:

For this reason, I propose a new redirection URI to redirect a End-User when it comes from a email link (from a verify mail or invitation mail). This URI would be a main page for the client, for example in example-app, http://127.0.0.1/

I'm sure I'm missing something, but how is this different than the http://coreos.com/invitation/callback? claim?

Are you trying to make it so that someone can log-in with the link in the emails?

@rsoletob
Copy link
Contributor Author

Hey there @bobbyrullo, our proposal is redirect user when it clicks on the email link to a place where client could do some OIDC flow, not give a valid authorization code throught email link.

I'm sure I'm missing something, but how is this different than the http://coreos.com/invitation/callback? claim?

This claim fit prefectly with our proposal, so when user is created, for example, client puts into request its main page to redirect, in example-app would be http://127.0.0.1. Therefore email link has main page URI, so if dex checks this redirect URI is valid, client would have this redirect URI registered in dex (client would has registered two redirect URIs: http://127.0.0.1/ and http://127.0.0.1/callback).

Are you trying to make it so that someone can log-in with the link in the emails?

Yes, we were trying it, here is the proposal using only http://127.0.0.1/callback (it needs improve error messages).

However, we prefer redirect user to main page (http://127.0.0.1/) instead of callback URI (http://127.0.0.1/callback).

What do you think about this?

@bobbyrullo
Copy link
Contributor

Ah. I think I finally understand what you are wanting - you want to be able to add state and nonce to the http://coreos.com/email/verification-callback so that when someone come back to the client after verifying, the client can recognize them, securely?

@miguelcubillo
Copy link
Contributor

Hi, I think what my colleague is trying to say is that although we'd love the idea of having a valid token that could be recognized by the client, we can't think of a secure way to generate that token. Without interaction with the client, we can't be sure that a state-less token (or invented, or provided by the client during the invite, possibly long time ago) will be accepted by the client.
Thus, our idea is just to redirect the user to somewhere different than the client's callback URI without a session (we definitely could use the http://coreos.com/invitation/callback claim to do so). From there, the user would have to start a standard login process. This is more annoying for the end user but is a more secure and client independent solution.

@bobbyrullo
Copy link
Contributor

@miguelcubillo I think I get you, so why not just register multiple redirect URLs with the client, and when creating a user, specify whatever URL you like in the UserCreateRequest (which will then get propogated to the http://coreos.com/email/verification-callback inside the verification token)

@rsoletob
Copy link
Contributor Author

Thanks for your help with this issue. If we register clients with two redirect URIs (http://localhost:5555/callback and http://localhost:5555/) we can redirect user where we want, in our case the main page. So I'll just close this one out then.

@bobbyrullo
Copy link
Contributor

great - glad we sorted that out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants