Skip to content

Commit

Permalink
Fixes for Refresh Tokens (#162)
Browse files Browse the repository at this point in the history
Not done enough testing with non-Google evidently.  This fixes a bunch
of issues with providers that don't support refresh tokens.  And also
fixes Google that got broken somewhere along the way.
  • Loading branch information
spjmurray authored Jan 28, 2025
1 parent a0cf760 commit 0cf06dc
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 15 deletions.
4 changes: 2 additions & 2 deletions charts/identity/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ description: A Helm chart for deploying Unikorn's IdP

type: application

version: v0.2.52-rc5
appVersion: v0.2.52-rc5
version: v0.2.52-rc6
appVersion: v0.2.52-rc6

icon: https://raw.githubusercontent.com/unikorn-cloud/assets/main/images/logos/dark-on-light/icon.png

Expand Down
27 changes: 18 additions & 9 deletions pkg/oauth2/oauth2.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,10 @@ type Code struct {
ClientScope Scope `json:"csc,omitempty"`
// ClientNonce is injected into a OIDC id_token.
ClientNonce string `json:"cno,omitempty"`
// AccessToken is the user's access token.
AccessToken string `json:"at"`
// RefreshToken is the users's refresh token.
RefreshToken string `json:"rt"`
// IDToken is the full set of claims returned by the provider.
IDToken *oidc.IDToken `json:"idt"`
// AccessTokenExpiry tells us how long the token will last for.
Expand Down Expand Up @@ -719,6 +723,8 @@ func (a *Authenticator) Callback(w http.ResponseWriter, r *http.Request) {
ClientScope: state.ClientScope,
ClientNonce: state.ClientNonce,
OAuth2Provider: state.OAuth2Provider,
AccessToken: tokens.AccessToken,
RefreshToken: tokens.RefreshToken,
IDToken: idToken,
AccessTokenExpiry: tokens.Expiry,
}
Expand Down Expand Up @@ -849,8 +855,10 @@ func (a *Authenticator) TokenAuthorizationCode(w http.ResponseWriter, r *http.Re
Subject: code.IDToken.Email.Email,
ClientID: code.ClientID,
Federated: &Federated{
Provider: code.OAuth2Provider,
Expiry: code.AccessTokenExpiry,
Provider: code.OAuth2Provider,
AccessToken: code.AccessToken,
RefreshToken: code.RefreshToken,
Expiry: code.AccessTokenExpiry,
},
}

Expand Down Expand Up @@ -918,22 +926,23 @@ func (a *Authenticator) TokenRefreshToken(w http.ResponseWriter, r *http.Request
return nil, err
}

if providerTokens.RefreshToken == "" {
return nil, errors.OAuth2ServerError("provider didn't supply a refresh token on refresh")
}

info := &IssueInfo{
Issuer: "https://" + r.Host,
Audience: r.Host,
Subject: claims.Claims.Subject,
ClientID: claims.Custom.ClientID,
Federated: &Federated{
Provider: claims.Custom.Provider,
Expiry: providerTokens.Expiry,
AccessToken: providerTokens.AccessToken,
Provider: claims.Custom.Provider,
AccessToken: providerTokens.AccessToken,
RefreshToken: providerTokens.RefreshToken,
Expiry: providerTokens.Expiry,
},
}

if providerTokens.RefreshToken != "" {
info.Federated.RefreshToken = &providerTokens.RefreshToken
}

tokens, err := a.Issue(r.Context(), info)
if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion pkg/oauth2/oauth2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func TestTokens(t *testing.T) {
Subject: "[email protected]",
Federated: &oauth2.Federated{
AccessToken: "foo",
RefreshToken: &refreshToken,
RefreshToken: refreshToken,
Expiry: time.Now().Add(2 * accessTokenDuration),
},
}
Expand Down
9 changes: 6 additions & 3 deletions pkg/oauth2/tokens.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ type Federated struct {
Provider string
Expiry time.Time
AccessToken string
RefreshToken *string
RefreshToken string
}

// ServiceAccount is any information required to issue a service account access token.
Expand Down Expand Up @@ -225,7 +225,10 @@ func (a *Authenticator) Issue(ctx context.Context, info *IssueInfo) (*Tokens, er
Expiry: expiry,
}

if info.Federated != nil && info.Federated.RefreshToken != nil {
// Only supply a refresh token if the provider provided one. We can then
// guarantee that all calls to refresh will then reauthenticate against
// the provider.
if info.Federated != nil && info.Federated.RefreshToken != "" {
rtClaims := &RefreshTokenClaims{
Claims: jwt.Claims{
ID: uuid.New().String(),
Expand All @@ -241,7 +244,7 @@ func (a *Authenticator) Issue(ctx context.Context, info *IssueInfo) (*Tokens, er
Custom: &CustomRefreshTokenClaims{
Provider: info.Federated.Provider,
ClientID: info.ClientID,
RefreshToken: *info.Federated.RefreshToken,
RefreshToken: info.Federated.RefreshToken,
},
}

Expand Down

0 comments on commit 0cf06dc

Please sign in to comment.