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

oauth2: allow token revocation without knowing the token (i.e. per user) #304

Closed
joelpickup opened this issue Nov 8, 2016 · 22 comments
Closed
Milestone

Comments

@joelpickup
Copy link
Contributor

@joelpickup joelpickup commented Nov 8, 2016

We would love a mechanism for the revocation of all tokens associated with a specific user. This would allow us to suspend user accounts from our admin panel.

@aeneasr aeneasr added the enhancement label Nov 8, 2016
@aeneasr

This comment has been minimized.

Copy link
Member

@aeneasr aeneasr commented Nov 8, 2016

if you have any proposals or ideas how to achieve that feel free to share them here!

@aeneasr

This comment has been minimized.

Copy link
Member

@aeneasr aeneasr commented Nov 18, 2016

I'm still not sure how to resolve this properly. Maybe only for priviledged clients?

@20zinnm

This comment has been minimized.

Copy link

@20zinnm 20zinnm commented Nov 28, 2016

How about having some memory-based messaging channel or whatnot between the auth servers and the resource servers such that, if the auth server revokes access for a user, it sends out a message to resource servers to reject tokens issued for a user from before the message came out? (I.e. some RabbitMQ deal?)

type MessagingService interface {
    RevokeAccess(user string) // Sends a message to everyone saying "this guy's tokens are invalid"
    MinimumIssuedAt(user string) (int64, error) // Returns the earliest time a token for this user should be issued at and be valid, or 0 if there has been no recall.
}

type MemoryMessaging struct {
    revocations map[string]int64
}
func (mm *MemoryMessaging) RevokeAccess(user string) {
    mm.revocations[user] = time.Now().UTC()
}
func (mm *MemoryMessaging) MinimumIssuedAt(user string) (int64, error) {
    a, _ := mm.revocations[user]
    return a || 0
}

// Somewhere else
var ms MessagingService
func IsValid(token Token) {
    mia, err := ms.MinimumIssuedAt(token.Subject)
    if mia > token.IssuedAt {
        // Token is pre-revoke-all
    }
}

Otherwise, I'd handle this the OAuth2 way--issue short-lived access tokens and longer refresh tokens.

@aeneasr

This comment has been minimized.

Copy link
Member

@aeneasr aeneasr commented Nov 29, 2016

Hm a message queue would add another depedency (what if you don't want to use rabbitmq). The most obvious way is to write a small SQL script that scans the subject and audience from the table and deletes the rows.

I think there is no way around a dedicated endpoint that is protected, similar like the JWK endpoint.

@aeneasr aeneasr added the oauth2 label Jun 5, 2017
@joshuarubin

This comment has been minimized.

Copy link

@joshuarubin joshuarubin commented Jun 27, 2017

I'm just curious if a solution to this issue is required to do the following:

  1. End user is taken through the user auth flow and approves the RequestedScopes for a single "app" (ClientID) which include the offline scope. User may never have access to the token or refresh_token.
  2. User later would like to revoke their approval for that particular "app".

As I understand it, this would only be possible if the "app" provides a mechanism for the user to do this since the app (which has access to the tokens) can revoke the token and refresh_token with the /oauth2/revoke endpoint.

We, however, would like to have a single interface for the user to see all the applications they have approved and have an opportunity to revoke access for any particular app.

We keep track of the applications (claims.Audience) that the user has approved by hooking into the consent app. So listing the applications for the user is not an issue. We just don't appear to have any way to revoke the refresh_token for that application/user without having it in hand.

@caseyf

This comment has been minimized.

Copy link

@caseyf caseyf commented Oct 19, 2017

Could this be added as a REST API that requires rn:hydra:whatever permissions?

My use case is the usual:

  • users need to be able to revoke permission that they have granted from a web dashboard outside of any client apps
  • staff needs to be able to revoke all tokens for a specific client + user combination
@aeneasr

This comment has been minimized.

Copy link
Member

@aeneasr aeneasr commented Oct 19, 2017

Yes, this should in fact be a REST API in Hydra which is protected by ACL!

@caseyf

This comment has been minimized.

Copy link

@caseyf caseyf commented Oct 19, 2017

In the meantime, can I safely delete records from hydra_oauth2_refresh and hydra_oauth2_access as a workaround?

@aeneasr

This comment has been minimized.

Copy link
Member

@aeneasr aeneasr commented Oct 19, 2017

Absolutely!

@pnicolcev-tulipretail

This comment has been minimized.

Copy link
Contributor

@pnicolcev-tulipretail pnicolcev-tulipretail commented Dec 22, 2017

same situation here for implementing a logout endpoint. I believe we'll go with dipping into the table as well for now.

@aeneasr

This comment has been minimized.

Copy link
Member

@aeneasr aeneasr commented Dec 23, 2017

I think the best way to support log out capabilities is by implementing OpenID Connect's Session Management Specs, specifically section 5. What do you think about this?

@pnicolcev-tulipretail

This comment has been minimized.

Copy link
Contributor

@pnicolcev-tulipretail pnicolcev-tulipretail commented Dec 27, 2017

That will do the trick for user logouts but we also have server side events, like disabling of users, that we'd like to trigger revocation by subject. I don't think section 5 would help us out with that. In our case we'd probably only have the subject and client id (maybe not even the client id).

If you implement the OpenID spec, would you be bouncing users to the consent app to perform operations on logout? We may want the flexibility to trigger events on logouts. If the consent app gets handed control and an api call to revoke tokens, we can implement our own events system (like I think what @20zinnm was asking for).

@aeneasr

This comment has been minimized.

Copy link
Member

@aeneasr aeneasr commented Dec 28, 2017

What OpenID Session Management Spec doesn't cover is the typical use case of revoking access of an application. I think that is also a valid aspect for this project, and I think it must also include revoking all granted access tokens per user.

Banning users however should be done by creating a global ban policy such as

{
  "resources": ["<.*>"],
  "actions": ["<.*>"],
  "subjects": ["users", "to", "ban"],
  "effect": "deny"
}

assuming that you use access control policies for AC in your system.

@aeneasr aeneasr mentioned this issue Feb 5, 2018
2 of 5 tasks complete
@MOZGIII

This comment has been minimized.

Copy link
Contributor

@MOZGIII MOZGIII commented Jun 12, 2018

We stumbled upon the need for this too.

Since it's not implemented yet, we'll be using a workaround solution: we have an auth gateway that accepts token from multiple sources (JWTs and opaque from hydra, I explained it at #248), and we'll add a blacklist of subs to it. Upon encountering a request from the blocked sub, we'll be denying the access, and also revoking the access token (since we'll have it in the request). So, does it seems reasonable?
One downside is we won't be able to anyhow invalidate a refresh token, because our gateway should actually never see it, but that's not a big deal, the access is granted via access tokens.
That said, without this feature, using hydra is practically as good as just using JWTs for our use case - at least in terms of session termination.

@arekkas, hope this feedback is useful, looking forward to discuss our approach if you spot any issues with it.

@aeneasr

This comment has been minimized.

Copy link
Member

@aeneasr aeneasr commented Jun 13, 2018

Absolutely, as stated here, in the forums, and the chat, this will make it into the 1.0 release (blocking, so it won't be pushed further back). Just need a bit of free time to work this out.

@MOZGIII

This comment has been minimized.

Copy link
Contributor

@MOZGIII MOZGIII commented Jun 13, 2018

@arekkas good to hear. Btw, is there any approach we could take to implement this now (without the changes to hydra itself)? For example, we can manually delete records from the database. If we just delete the session record, would hydra pick it up?

@aeneasr

This comment has been minimized.

Copy link
Member

@aeneasr aeneasr commented Jun 13, 2018

Yes, basically you'll have to query the consent tables (one for authN, one for consent) and delete the responsible rows. That's how the REST API will handle this as well.

@MOZGIII

This comment has been minimized.

Copy link
Contributor

@MOZGIII MOZGIII commented Jun 13, 2018

Great, thanks. So, it seems like something we could also easily backport to 0.11.12 when it's implemented. I don't think we will be upgrading to 1.0.0 at this point, as it adds complications with securing access to the hydra itself.

@aeneasr

This comment has been minimized.

Copy link
Member

@aeneasr aeneasr commented Jun 13, 2018

Since you're bringing this up, feel free to participate in #904 :)

@aeneasr

This comment has been minimized.

Copy link
Member

@aeneasr aeneasr commented Jun 13, 2018

By the way, I don't think you can easily backport this, we had to resolve a bug in fosite which was merged in 1.0.0 (but not 0.11). The bug caused us to lose the chain of tokens (from consent to first issuance, to refresh, to refresh, ...). Also, the internal consent management is vastly different in 1.0 when compared to 0.11.

My personal recommendation is to participate in #904 and raise your use case. We'll try to find a good trade-off between complexity and security. Once merged, you should definitely move to 1.0 as 0.11 will no longer be maintained without a paid support SLA.

@MOZGIII

This comment has been minimized.

Copy link
Contributor

@MOZGIII MOZGIII commented Jun 13, 2018

Got it, thanks for the info, I'll look into #904.

@aeneasr

This comment has been minimized.

Copy link
Member

@aeneasr aeneasr commented Jul 7, 2018

Fixed by #856

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.