There is a chance that offline storage could fall out of sync with the
refresh token tables. One example is if dex crashes/is stopped in the
middle of handling a login request. If the old refresh token associated
with the offline session is deleted, and then the process stops, the
offline session will still refer to the old token.
Unfortunately, if this case occurs, there is no way to recover from it,
since further logins will be halted due to dex being unable to clean up
the old tokens till referenced in the offline session: the database is
essentially corrupted.
There doesn't seem to be a good reason to fail the auth request if the
old refresh token is gone. This changes the logic in `handleAuthCode` to
not fail the entire transaction if the old refresh token could not be
deleted because it was not present. This has the effect of installing
the new refresh token, and unpdating the offline storage, thereby fixing
the issue, however it occured.
Now, we'll return a standard error, and have the caller act upon this
being an instance of authErr.
Also changes the storage.AuthRequest return to a pointer, and returns
nil in error cases.
Signed-off-by: Stephan Renatus <srenatus@chef.io>
Co-authored-by: Yuxing Li <360983+jackielii@users.noreply.github.com>
Co-authored-by: Francisco Santiago <1737357+fjbsantiago@users.noreply.github.com>
Before, you could not POST your credentials to a password-connector's
endpoint without GETing that endpoint first. While this makes sense for
browser clients; automated interactions with Dex don't need to look at
the password form to fill it in.
A symptom of that missing GET was that the POST succeeded (!) with
login successful: connector "", username="admin", email="admin@example.com", groups=[]
Note the connector "". A subsequent call to finalizeLogin would then
fail with
connector with ID "" not found: failed to get connector object from storage: not found
Now, the connector ID of an auth request will be updated for both GETs
and POSTs.
Signed-off-by: Stephan Renatus <srenatus@chef.io>
This way, the user who has selected, say, "Log in with Email" can make up
their mind, and select a different connector instead.
However, if there's only one connector set up, none of this makes sense -- and
the link will thus not be displayed.
Signed-off-by: Stephan Renatus <srenatus@chef.io>
This allows users of the LDAP connector to give users of Dex' login
prompt an idea of what they should enter for a username.
Before, irregardless of how the LDAP connector was set up, the prompt
was
Username
[_________________]
Password
[_________________]
Now, this is configurable, and can be used to say "MyCorp SSO Login" if
that's what it is.
If it's not configured, it will default to "Username".
For the passwordDB connector (local users), it is set to "Email
Address", since this is what it uses.
Signed-off-by: Stephan Renatus <srenatus@chef.io>
Switch from using "text/template" to "html/template", which provides
basic XSS preventions. We haven't identified any particular place
where unsanitized user data is rendered to the frontend. This is
just a preventative step.
At the same time, make more templates take pure URL instead of
forming an URL themselves using an "authReqID" argument. This will
help us stop using the auth req ID in certain places, preventing
garbage collection from killing login flows that wait too long at
the login screen.
Also increase the login session window (time between initial
redirect and the user logging in) from 30 minutes to 24 hours,
and display a more helpful error message when the session expires.
How to test:
1. Spin up dex and example with examples/config-dev.yaml.
2. Login through both the password prompt and the direct redirect.
3. Edit examples/config-dev.yaml removing the "connectors" section.
4. Ensure you can still login with a password.
(email/password is "admin@example.com" and "password")
The "at_hash" claim, which provides hash verification for the
"access_token," is a required claim for implicit and hybrid flow
requests. Previously we did not include it (against spec). This
PR implements the "at_hash" logic and adds the claim to all
responses.
As a cleanup, it also moves some JOSE signing logic out of the
storage package and into the server package.
For details see:
https://openid.net/specs/openid-connect-core-1_0.html#ImplicitIDToken
The server implements a strategy called "Refresh Token Rotation" to
ensure refresh tokens can only be claimed once.
ref: https://tools.ietf.org/html/rfc6819#section-5.2.2.3
Previously "refresh_token" values in token responses where just the
ID of the internal refresh object. To implement rotation, when a
client redeemed a refresh token, the object would be deleted, a new
one created, and the new ID returned as the new "refresh_token".
However, this means there was no consistent ID for refresh tokens
internally, making things like foreign keys very hard to implement.
This is problematic for revocation features like showing all the
refresh tokens a user or client has out.
This PR updates the "refresh_token" to be an encoded protobuf
message, which holds the internal ID and a nonce. When a refresh
token is used, the nonce is updated to prevent reuse, but the ID
remains the same. Additionally it adds the timestamp of each
token's last use.
Accept the following response_type for the implicit flow:
id_token
token id_token
And the following for hybrid flow
code id_token
code token
code token id_token
This corrects the previous behavior of the implicit flow, which
only accepted "token" (now correctly rejected).
"state" means something specific to OAuth2 and SAML so we don't
want to confuse developers who are working on this.
Also don't use "session" which could easily be confused with HTTP
cookies.
Let the server handle the state token instead of the connector. As a
result it can throw out bad requests earlier. It can also use that
token to determine which connector was used to generate the request
allowing all connectors to share the same callback URL.
Callbacks now all look like:
https://dex.example.com/callback
Instead of:
https://dex.example.com/callback/(connector id)
Even when multiple connectors are being used.
fixes: #636
This commit addresses a problem where the `max-age` value is being set
in nanoseconds as opposed to seconds, as required by the specification.
go-oidc sends an extra space before the list of scopes. This is bad
but we have to support it, so we'll be more lenient and ignore
duplicated whitespace.
Currently, whether or not a user has authenticated themselves through
a connector is indicated by a pointer being nil or non-nil. Instead
add an explicit flag that marks this.
