Don't squash profileURL session fields during refresh
This commit is contained in:
		
							parent
							
								
									98f8195902
								
							
						
					
					
						commit
						cbd4ce654e
					
				|  | @ -12,6 +12,8 @@ | ||||||
| ## Breaking Changes | ## Breaking Changes | ||||||
| 
 | 
 | ||||||
| ## Changes since v7.1.3 | ## Changes since v7.1.3 | ||||||
|  | 
 | ||||||
|  | - [#1251](https://github.com/oauth2-proxy/oauth2-proxy/pull/1251) Add safety checks to OIDC session fields during refresh (@NickMeves) | ||||||
| - [#1233](https://github.com/oauth2-proxy/oauth2-proxy/pull/1233) Extend email-domain validation with sub-domain capability (@morarucostel) | - [#1233](https://github.com/oauth2-proxy/oauth2-proxy/pull/1233) Extend email-domain validation with sub-domain capability (@morarucostel) | ||||||
| - [#1060](https://github.com/oauth2-proxy/oauth2-proxy/pull/1060) Implement RewriteTarget to allow requests to be rewritten before proxying to upstream servers (@JoelSpeed) | - [#1060](https://github.com/oauth2-proxy/oauth2-proxy/pull/1060) Implement RewriteTarget to allow requests to be rewritten before proxying to upstream servers (@JoelSpeed) | ||||||
| - [#1086](https://github.com/oauth2-proxy/oauth2-proxy/pull/1086) Refresh sessions before token expiration if configured (@NickMeves) | - [#1086](https://github.com/oauth2-proxy/oauth2-proxy/pull/1086) Refresh sessions before token expiration if configured (@NickMeves) | ||||||
|  |  | ||||||
|  | @ -186,14 +186,31 @@ func (p *OIDCProvider) redeemRefreshToken(ctx context.Context, s *sessions.Sessi | ||||||
| 		return fmt.Errorf("unable create new session state from response: %v", err) | 		return fmt.Errorf("unable create new session state from response: %v", err) | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
|  | 	replaceSession(s, newSession) | ||||||
|  | 	return nil | ||||||
|  | } | ||||||
|  | 
 | ||||||
|  | func replaceSession(s *sessions.SessionState, newSession *sessions.SessionState) { | ||||||
| 	// It's possible that if the refresh token isn't in the token response the
 | 	// It's possible that if the refresh token isn't in the token response the
 | ||||||
| 	// session will not contain an id token.
 | 	// session will not contain an id token.
 | ||||||
| 	// If it doesn't it's probably better to retain the old one
 | 	// If it doesn't it's probably better to retain the old one
 | ||||||
| 	if newSession.IDToken != "" { | 	if newSession.IDToken != "" { | ||||||
| 		s.IDToken = newSession.IDToken | 		s.IDToken = newSession.IDToken | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	// Only copy over fields if they are present. Otherwise they might've
 | ||||||
|  | 	// originally been set via a ProfileURL call in `EnrichSession` but
 | ||||||
|  | 	// are empty in IDToken claims.
 | ||||||
|  | 	if newSession.Email != "" { | ||||||
| 		s.Email = newSession.Email | 		s.Email = newSession.Email | ||||||
|  | 	} | ||||||
|  | 	if newSession.User != "" { | ||||||
| 		s.User = newSession.User | 		s.User = newSession.User | ||||||
|  | 	} | ||||||
|  | 	if newSession.Groups != nil { | ||||||
| 		s.Groups = newSession.Groups | 		s.Groups = newSession.Groups | ||||||
|  | 	} | ||||||
|  | 	if newSession.PreferredUsername != "" { | ||||||
| 		s.PreferredUsername = newSession.PreferredUsername | 		s.PreferredUsername = newSession.PreferredUsername | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
|  | @ -201,8 +218,6 @@ func (p *OIDCProvider) redeemRefreshToken(ctx context.Context, s *sessions.Sessi | ||||||
| 	s.RefreshToken = newSession.RefreshToken | 	s.RefreshToken = newSession.RefreshToken | ||||||
| 	s.CreatedAt = newSession.CreatedAt | 	s.CreatedAt = newSession.CreatedAt | ||||||
| 	s.ExpiresOn = newSession.ExpiresOn | 	s.ExpiresOn = newSession.ExpiresOn | ||||||
| 
 |  | ||||||
| 	return nil |  | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| // CreateSessionFromToken converts Bearer IDTokens into sessions
 | // CreateSessionFromToken converts Bearer IDTokens into sessions
 | ||||||
|  |  | ||||||
|  | @ -464,7 +464,7 @@ func TestOIDCProvider_EnrichSession(t *testing.T) { | ||||||
| 	} | 	} | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| func TestOIDCProviderRefreshSessionIfNeededWithoutIdToken(t *testing.T) { | func TestOIDCProviderRefreshSessionWithoutIdToken(t *testing.T) { | ||||||
| 
 | 
 | ||||||
| 	idToken, _ := newSignedTestIDToken(defaultIDToken) | 	idToken, _ := newSignedTestIDToken(defaultIDToken) | ||||||
| 	body, _ := json.Marshal(redeemTokenResponse{ | 	body, _ := json.Marshal(redeemTokenResponse{ | ||||||
|  | @ -497,7 +497,7 @@ func TestOIDCProviderRefreshSessionIfNeededWithoutIdToken(t *testing.T) { | ||||||
| 	assert.Equal(t, "11223344", existingSession.User) | 	assert.Equal(t, "11223344", existingSession.User) | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| func TestOIDCProviderRefreshSessionIfNeededWithIdToken(t *testing.T) { | func TestOIDCProviderRefreshSessionWithIdToken(t *testing.T) { | ||||||
| 
 | 
 | ||||||
| 	idToken, _ := newSignedTestIDToken(defaultIDToken) | 	idToken, _ := newSignedTestIDToken(defaultIDToken) | ||||||
| 	body, _ := json.Marshal(redeemTokenResponse{ | 	body, _ := json.Marshal(redeemTokenResponse{ | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue