api: adding a gRPC call for listing refresh tokens. #801
Conversation
api/api.proto
Outdated
|
|
||
| // ListRefreshReq is a request to enumerate the refresh tokens of a user. | ||
| message ListRefreshReq { | ||
| string user_id = 1; |
There was a problem hiding this comment.
per IRL conversation the users of this API wont have the user_id and conn_id. only the subject from the id_token. Right now the subject is just the user_id[0].
Just like we did for refresh tokens, we need to embed more information in the id token's subject. We need to add the conn_id.
I'm going to advocate that we make another internal struct like we did for refresh tokens[1], then mimic the serialization.[2]
[0] https://github.com/coreos/dex/blob/v2.1.0/server/oauth2.go#L270
[1] https://github.com/coreos/dex/blob/v2.1.0/server/internal/types.proto
[2] https://github.com/coreos/dex/blob/v2.1.0/server/handlers.go#L656-L664
There was a problem hiding this comment.
Updated the id token's subject to have the conn_id along with the user_id.
There was a problem hiding this comment.
nit: need to update this struct to only take a user_id. Maybe a comment like this?
message ListRefreshReq {
// The "sub" claim returned in the ID Token
string user_id = 1;
}
rithujohn191
force-pushed
the
token-revocation
branch
from
6ced1be
to
e558853
Compare
server/internal/types.proto
Outdated
|
|
||
| // OfflineSessionID represents both the userID and connID required to | ||
| // uniquely identify an OfflineSession object | ||
| message OfflineSessionID { |
There was a problem hiding this comment.
nit: Can we just call this IDTokenSubject or something? That's actually what we're using this for.
api/api.proto
Outdated
|
|
||
| // ListRefreshReq is a request to enumerate the refresh tokens of a user. | ||
| message ListRefreshReq { | ||
| string user_id = 1; |
There was a problem hiding this comment.
nit: need to update this struct to only take a user_id. Maybe a comment like this?
message ListRefreshReq {
// The "sub" claim returned in the ID Token
string user_id = 1;
}
server/oauth2.go
Outdated
| @@ -246,7 +247,7 @@ type idTokenClaims struct { | |||
| Name string `json:"name,omitempty"` | |||
| } | |||
|
|
|||
| func (s *Server) newIDToken(clientID string, claims storage.Claims, scopes []string, nonce, accessToken string) (idToken string, expiry time.Time, err error) { | |||
| func (s *Server) newIDToken(clientID string, claims storage.Claims, scopes []string, nonce, accessToken string, connID string) (idToken string, expiry time.Time, err error) { | |||
There was a problem hiding this comment.
nit: the accessToken string, is redundant because connID is a string. You can just do accessToken, connID string
rithujohn191
force-pushed
the
token-revocation
branch
from
e558853
to
d201e49
Compare
|
Addressed all the feedback. Fixed all the nits. |
|
sick, lgtm |
|
oh wait, we need to bump the API version. Maybe we can just do that when we add the revocation calls? |
|
Yep will bump the API version when I add the calls for revocation. |
This call enables us to list refresh tokens for a user by providing the UserID and ConnID. This will be helpful when we want to enable refresh token revocation based on ClientIDs.