From 4debb74edada3a415f672cd03ee51a5b3b215c00 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Thu, 19 Aug 2021 18:11:30 +0200 Subject: [PATCH] Fix wrong user in OpenID response (#16736) * Fixed usage of wrong user. * Added tests. --- models/fixtures/oauth2_grant.yml | 16 +++++++ routers/web/user/oauth.go | 20 ++++----- routers/web/user/oauth_test.go | 75 ++++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 10 deletions(-) create mode 100644 routers/web/user/oauth_test.go diff --git a/models/fixtures/oauth2_grant.yml b/models/fixtures/oauth2_grant.yml index 105e3f22db..e52a2bce95 100644 --- a/models/fixtures/oauth2_grant.yml +++ b/models/fixtures/oauth2_grant.yml @@ -5,3 +5,19 @@ scope: "openid profile" created_unix: 1546869730 updated_unix: 1546869730 + +- id: 2 + user_id: 3 + application_id: 1 + counter: 1 + scope: "openid" + created_unix: 1546869730 + updated_unix: 1546869730 + +- id: 3 + user_id: 5 + application_id: 1 + counter: 1 + scope: "openid profile email" + created_unix: 1546869730 + updated_unix: 1546869730 \ No newline at end of file diff --git a/routers/web/user/oauth.go b/routers/web/user/oauth.go index 18fc943483..67e4ea0622 100644 --- a/routers/web/user/oauth.go +++ b/routers/web/user/oauth.go @@ -187,7 +187,7 @@ func newAccessTokenResponse(grant *models.OAuth2Grant, signingKey oauth2.JWTSign ErrorDescription: "cannot find application", } } - err = app.LoadUser() + user, err := models.GetUserByID(grant.UserID) if err != nil { if models.IsErrUserNotExist(err) { return nil, &AccessTokenError{ @@ -212,17 +212,17 @@ func newAccessTokenResponse(grant *models.OAuth2Grant, signingKey oauth2.JWTSign Nonce: grant.Nonce, } if grant.ScopeContains("profile") { - idToken.Name = app.User.FullName - idToken.PreferredUsername = app.User.Name - idToken.Profile = app.User.HTMLURL() - idToken.Picture = app.User.AvatarLink() - idToken.Website = app.User.Website - idToken.Locale = app.User.Language - idToken.UpdatedAt = app.User.UpdatedUnix + idToken.Name = user.FullName + idToken.PreferredUsername = user.Name + idToken.Profile = user.HTMLURL() + idToken.Picture = user.AvatarLink() + idToken.Website = user.Website + idToken.Locale = user.Language + idToken.UpdatedAt = user.UpdatedUnix } if grant.ScopeContains("email") { - idToken.Email = app.User.Email - idToken.EmailVerified = app.User.IsActive + idToken.Email = user.Email + idToken.EmailVerified = user.IsActive } signedIDToken, err = idToken.SignToken(signingKey) diff --git a/routers/web/user/oauth_test.go b/routers/web/user/oauth_test.go new file mode 100644 index 0000000000..0283431960 --- /dev/null +++ b/routers/web/user/oauth_test.go @@ -0,0 +1,75 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package user + +import ( + "testing" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/services/auth/source/oauth2" + + "github.com/golang-jwt/jwt" + "github.com/stretchr/testify/assert" +) + +func createAndParseToken(t *testing.T, grant *models.OAuth2Grant) *oauth2.OIDCToken { + signingKey, err := oauth2.CreateJWTSingingKey("HS256", make([]byte, 32)) + assert.NoError(t, err) + assert.NotNil(t, signingKey) + oauth2.DefaultSigningKey = signingKey + + response, terr := newAccessTokenResponse(grant, signingKey) + assert.Nil(t, terr) + assert.NotNil(t, response) + + parsedToken, err := jwt.ParseWithClaims(response.IDToken, &oauth2.OIDCToken{}, func(token *jwt.Token) (interface{}, error) { + assert.NotNil(t, token.Method) + assert.Equal(t, signingKey.SigningMethod().Alg(), token.Method.Alg()) + return signingKey.VerifyKey(), nil + }) + assert.NoError(t, err) + assert.True(t, parsedToken.Valid) + + oidcToken, ok := parsedToken.Claims.(*oauth2.OIDCToken) + assert.True(t, ok) + assert.NotNil(t, oidcToken) + + return oidcToken +} + +func TestNewAccessTokenResponse_OIDCToken(t *testing.T) { + assert.NoError(t, models.PrepareTestDatabase()) + + grants, err := models.GetOAuth2GrantsByUserID(3) + assert.NoError(t, err) + assert.Len(t, grants, 1) + + // Scopes: openid + oidcToken := createAndParseToken(t, grants[0]) + assert.Empty(t, oidcToken.Name) + assert.Empty(t, oidcToken.PreferredUsername) + assert.Empty(t, oidcToken.Profile) + assert.Empty(t, oidcToken.Picture) + assert.Empty(t, oidcToken.Website) + assert.Empty(t, oidcToken.UpdatedAt) + assert.Empty(t, oidcToken.Email) + assert.False(t, oidcToken.EmailVerified) + + user := models.AssertExistsAndLoadBean(t, &models.User{ID: 5}).(*models.User) + grants, err = models.GetOAuth2GrantsByUserID(user.ID) + assert.NoError(t, err) + assert.Len(t, grants, 1) + + // Scopes: openid profile email + oidcToken = createAndParseToken(t, grants[0]) + assert.Equal(t, user.FullName, oidcToken.Name) + assert.Equal(t, user.Name, oidcToken.PreferredUsername) + assert.Equal(t, user.HTMLURL(), oidcToken.Profile) + assert.Equal(t, user.AvatarLink(), oidcToken.Picture) + assert.Equal(t, user.Website, oidcToken.Website) + assert.Equal(t, user.UpdatedUnix, oidcToken.UpdatedAt) + assert.Equal(t, user.Email, oidcToken.Email) + assert.Equal(t, user.IsActive, oidcToken.EmailVerified) +}