From c915d45fbc4bcb7ecc35b0d6adf7ea89b30293e8 Mon Sep 17 00:00:00 2001 From: Nils Hillmann Date: Mon, 3 May 2021 09:14:07 +0200 Subject: [PATCH 1/8] Implemented userinfo #8534 --- routers/routes/web.go | 1 + routers/user/oauth.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/routers/routes/web.go b/routers/routes/web.go index 72f5c27b6f865..2e79d00354568 100644 --- a/routers/routes/web.go +++ b/routers/routes/web.go @@ -401,6 +401,7 @@ func RegisterRoutes(m *web.Route) { // TODO manage redirection m.Post("/authorize", bindIgnErr(forms.AuthorizationForm{}), user.AuthorizeOAuth) }, ignSignInAndCsrf, reqSignIn) + m.Get("/login/oauth/userinfo", ignSignInAndCsrf, user.UserInfoOAuth) if setting.CORSConfig.Enabled { m.Post("/login/oauth/access_token", cors.Handler(cors.Options{ //Scheme: setting.CORSConfig.Scheme, // FIXME: the cors middleware needs scheme option diff --git a/routers/user/oauth.go b/routers/user/oauth.go index ae06efd0c0168..4206c0514d9a7 100644 --- a/routers/user/oauth.go +++ b/routers/user/oauth.go @@ -13,6 +13,7 @@ import ( "strings" "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/auth/sso" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" @@ -193,6 +194,39 @@ func newAccessTokenResponse(grant *models.OAuth2Grant, clientSecret string) (*Ac }, nil } +type userInfoResponse struct { + Sub string `json:"sub"` + Name string `json:"name"` + Username string `json:"preferred_username"` + Email string `json:"email"` + Picture string `json:"picture"` +} + +func UserInfoOAuth(ctx *context.Context) { + header := ctx.Req.Header.Get("Authorization") + auths := strings.Fields(header) + if len(auths) != 2 || auths[0] != "Bearer" { + ctx.HandleText(http.StatusUnauthorized, "no valid auth token authorization") + return + } + uid := sso.CheckOAuthAccessToken(auths[1]) + if uid != 0 { + authUser, err := models.GetUserByID(uid) + if err != nil { + ctx.ServerError("GetUserByID", err) + return + } + response := &userInfoResponse{ + Sub: fmt.Sprint(authUser.ID), + Name: authUser.FullName, + Username: authUser.Name, + Email: authUser.Email, + Picture: authUser.AvatarLink(), + } + ctx.JSON(http.StatusOK, response) + } +} + // AuthorizeOAuth manages authorize requests func AuthorizeOAuth(ctx *context.Context) { form := web.GetForm(ctx).(*forms.AuthorizationForm) From 7964ae85c202ad0e4bd059659a5d46a78dbf1e26 Mon Sep 17 00:00:00 2001 From: "Nils L. Hillmann" Date: Tue, 4 May 2021 19:38:52 +0200 Subject: [PATCH 2/8] Make lint happy --- routers/routes/web.go | 2 +- routers/user/oauth.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/routers/routes/web.go b/routers/routes/web.go index 2e79d00354568..b792c7a6cf426 100644 --- a/routers/routes/web.go +++ b/routers/routes/web.go @@ -401,7 +401,7 @@ func RegisterRoutes(m *web.Route) { // TODO manage redirection m.Post("/authorize", bindIgnErr(forms.AuthorizationForm{}), user.AuthorizeOAuth) }, ignSignInAndCsrf, reqSignIn) - m.Get("/login/oauth/userinfo", ignSignInAndCsrf, user.UserInfoOAuth) + m.Get("/login/oauth/userinfo", ignSignInAndCsrf, user.InfoOAuth) if setting.CORSConfig.Enabled { m.Post("/login/oauth/access_token", cors.Handler(cors.Options{ //Scheme: setting.CORSConfig.Scheme, // FIXME: the cors middleware needs scheme option diff --git a/routers/user/oauth.go b/routers/user/oauth.go index 4206c0514d9a7..442f1af690116 100644 --- a/routers/user/oauth.go +++ b/routers/user/oauth.go @@ -202,7 +202,8 @@ type userInfoResponse struct { Picture string `json:"picture"` } -func UserInfoOAuth(ctx *context.Context) { +// InfoOAuth manages request for userinfo endpoint +func InfoOAuth(ctx *context.Context) { header := ctx.Req.Header.Get("Authorization") auths := strings.Fields(header) if len(auths) != 2 || auths[0] != "Bearer" { From 8b65e35b8e3842e85ea4aa23021108776ee1c41b Mon Sep 17 00:00:00 2001 From: "Nils L. Hillmann" Date: Tue, 4 May 2021 19:39:17 +0200 Subject: [PATCH 3/8] Add userinfo endpoint to openid-configuration --- templates/user/auth/oidc_wellknown.tmpl | 1 + 1 file changed, 1 insertion(+) diff --git a/templates/user/auth/oidc_wellknown.tmpl b/templates/user/auth/oidc_wellknown.tmpl index 290ed4a71df40..fcde060a8d19f 100644 --- a/templates/user/auth/oidc_wellknown.tmpl +++ b/templates/user/auth/oidc_wellknown.tmpl @@ -2,6 +2,7 @@ "issuer": "{{AppUrl | JSEscape | Safe}}", "authorization_endpoint": "{{AppUrl | JSEscape | Safe}}login/oauth/authorize", "token_endpoint": "{{AppUrl | JSEscape | Safe}}login/oauth/access_token", + "userinfo_endpoint": "{{AppUrl | JSEscape | Safe}}login/oauth/userinfo", "response_types_supported": [ "code", "id_token" From 1c36c9920b853a02f0d0380316f633a42de8fd01 Mon Sep 17 00:00:00 2001 From: "Nils L. Hillmann" Date: Wed, 5 May 2021 16:25:42 +0200 Subject: [PATCH 4/8] Give an error when uid equals 0 --- routers/user/oauth.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/routers/user/oauth.go b/routers/user/oauth.go index 442f1af690116..7b561356de61f 100644 --- a/routers/user/oauth.go +++ b/routers/user/oauth.go @@ -225,6 +225,8 @@ func InfoOAuth(ctx *context.Context) { Picture: authUser.AvatarLink(), } ctx.JSON(http.StatusOK, response) + } else { + ctx.ServerError("InfoOAuth:", fmt.Errorf("UserID not valid")) } } From a4dd8fc812db666efdd3b776ceec34d19585f5fc Mon Sep 17 00:00:00 2001 From: Nils Hillmann Date: Wed, 5 May 2021 18:58:14 +0200 Subject: [PATCH 5/8] Implemented BearerTokenErrorCode handling --- routers/user/oauth.go | 40 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/routers/user/oauth.go b/routers/user/oauth.go index 7b561356de61f..7ae9dd8c20dbe 100644 --- a/routers/user/oauth.go +++ b/routers/user/oauth.go @@ -94,6 +94,24 @@ func (err AccessTokenError) Error() string { return fmt.Sprintf("%s: %s", err.ErrorCode, err.ErrorDescription) } +// BearerTokenErrorCode represents an error code specified in RFC 6750 +type BearerTokenErrorCode string + +const ( + // BearerTokenErrorCodeInvalidRequest represents an error code specified in RFC 6750 + BearerTokenErrorCodeInvalidRequest BearerTokenErrorCode = "invalid_request" + // BearerTokenErrorCodeInvalidToken represents an error code specified in RFC 6750 + BearerTokenErrorCodeInvalidToken BearerTokenErrorCode = "invalid_token" + // BearerTokenErrorCodeInsufficientScope represents an error code specified in RFC 6750 + BearerTokenErrorCodeInsufficientScope BearerTokenErrorCode = "insufficient_scope" +) + +// BearerTokenError represents an error response specified in RFC 6750 +type BearerTokenError struct { + ErrorCode BearerTokenErrorCode + ErrorDescription string +} + // TokenType specifies the kind of token type TokenType string @@ -211,6 +229,13 @@ func InfoOAuth(ctx *context.Context) { return } uid := sso.CheckOAuthAccessToken(auths[1]) + if uid == 0 { + handleBearerTokenError(ctx, BearerTokenError{ + ErrorCode: BearerTokenErrorCodeInvalidToken, + ErrorDescription: "Access token not assigned to any user", + }) + return + } if uid != 0 { authUser, err := models.GetUserByID(uid) if err != nil { @@ -225,8 +250,6 @@ func InfoOAuth(ctx *context.Context) { Picture: authUser.AvatarLink(), } ctx.JSON(http.StatusOK, response) - } else { - ctx.ServerError("InfoOAuth:", fmt.Errorf("UserID not valid")) } } @@ -608,3 +631,16 @@ func handleAuthorizeError(ctx *context.Context, authErr AuthorizeError, redirect redirect.RawQuery = q.Encode() ctx.Redirect(redirect.String(), 302) } + +func handleBearerTokenError(ctx *context.Context, beErr BearerTokenError) { + ctx.Resp.Header().Set("WWW-Authenticate", fmt.Sprintf("Bearer realm=\"\", error=\"%s\", error_description=\"%s\"", beErr.ErrorCode, beErr.ErrorDescription)) + if beErr.ErrorCode == BearerTokenErrorCodeInvalidRequest { + ctx.Error(http.StatusBadRequest) + } + if beErr.ErrorCode == BearerTokenErrorCodeInvalidToken { + ctx.Error(http.StatusUnauthorized) + } + if beErr.ErrorCode == BearerTokenErrorCodeInsufficientScope { + ctx.Error(http.StatusForbidden) + } +} From d25562814d4a92e4f0373f409f0085e0f5bf6bd5 Mon Sep 17 00:00:00 2001 From: Nils Hillmann Date: Wed, 5 May 2021 19:27:06 +0200 Subject: [PATCH 6/8] instead of ctx.error use ctx.json so that clients parse error and error_description correctly --- routers/user/oauth.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/routers/user/oauth.go b/routers/user/oauth.go index 7ae9dd8c20dbe..ea13362c18f16 100644 --- a/routers/user/oauth.go +++ b/routers/user/oauth.go @@ -108,8 +108,8 @@ const ( // BearerTokenError represents an error response specified in RFC 6750 type BearerTokenError struct { - ErrorCode BearerTokenErrorCode - ErrorDescription string + ErrorCode BearerTokenErrorCode `json:"error" form:"error"` + ErrorDescription string `json:"error_description"` } // TokenType specifies the kind of token @@ -635,12 +635,12 @@ func handleAuthorizeError(ctx *context.Context, authErr AuthorizeError, redirect func handleBearerTokenError(ctx *context.Context, beErr BearerTokenError) { ctx.Resp.Header().Set("WWW-Authenticate", fmt.Sprintf("Bearer realm=\"\", error=\"%s\", error_description=\"%s\"", beErr.ErrorCode, beErr.ErrorDescription)) if beErr.ErrorCode == BearerTokenErrorCodeInvalidRequest { - ctx.Error(http.StatusBadRequest) + ctx.JSON(http.StatusBadRequest, beErr) } if beErr.ErrorCode == BearerTokenErrorCodeInvalidToken { - ctx.Error(http.StatusUnauthorized) + ctx.JSON(http.StatusUnauthorized, beErr) } if beErr.ErrorCode == BearerTokenErrorCodeInsufficientScope { - ctx.Error(http.StatusForbidden) + ctx.JSON(http.StatusForbidden, beErr) } } From 394def7574f0131016af1536f316b7c8364180b2 Mon Sep 17 00:00:00 2001 From: "Nils L. Hillmann" Date: Thu, 6 May 2021 06:35:01 +0200 Subject: [PATCH 7/8] Removed unneeded if statement --- routers/user/oauth.go | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/routers/user/oauth.go b/routers/user/oauth.go index ea13362c18f16..42b7b7bafc3df 100644 --- a/routers/user/oauth.go +++ b/routers/user/oauth.go @@ -236,21 +236,19 @@ func InfoOAuth(ctx *context.Context) { }) return } - if uid != 0 { - authUser, err := models.GetUserByID(uid) - if err != nil { - ctx.ServerError("GetUserByID", err) - return - } - response := &userInfoResponse{ - Sub: fmt.Sprint(authUser.ID), - Name: authUser.FullName, - Username: authUser.Name, - Email: authUser.Email, - Picture: authUser.AvatarLink(), - } - ctx.JSON(http.StatusOK, response) + authUser, err := models.GetUserByID(uid) + if err != nil { + ctx.ServerError("GetUserByID", err) + return + } + response := &userInfoResponse{ + Sub: fmt.Sprint(authUser.ID), + Name: authUser.FullName, + Username: authUser.Name, + Email: authUser.Email, + Picture: authUser.AvatarLink(), } + ctx.JSON(http.StatusOK, response) } // AuthorizeOAuth manages authorize requests From f76f63eb7606f0420b34e2165260df997230fb5b Mon Sep 17 00:00:00 2001 From: "Nils L. Hillmann" Date: Thu, 6 May 2021 06:46:56 +0200 Subject: [PATCH 8/8] Use switch instead of subsequent if statements Have a default for unknown errorcodes. --- routers/user/oauth.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/routers/user/oauth.go b/routers/user/oauth.go index 42b7b7bafc3df..3ef5a56c01734 100644 --- a/routers/user/oauth.go +++ b/routers/user/oauth.go @@ -632,13 +632,15 @@ func handleAuthorizeError(ctx *context.Context, authErr AuthorizeError, redirect func handleBearerTokenError(ctx *context.Context, beErr BearerTokenError) { ctx.Resp.Header().Set("WWW-Authenticate", fmt.Sprintf("Bearer realm=\"\", error=\"%s\", error_description=\"%s\"", beErr.ErrorCode, beErr.ErrorDescription)) - if beErr.ErrorCode == BearerTokenErrorCodeInvalidRequest { + switch beErr.ErrorCode { + case BearerTokenErrorCodeInvalidRequest: ctx.JSON(http.StatusBadRequest, beErr) - } - if beErr.ErrorCode == BearerTokenErrorCodeInvalidToken { + case BearerTokenErrorCodeInvalidToken: ctx.JSON(http.StatusUnauthorized, beErr) - } - if beErr.ErrorCode == BearerTokenErrorCodeInsufficientScope { + case BearerTokenErrorCodeInsufficientScope: ctx.JSON(http.StatusForbidden, beErr) + default: + log.Error("Invalid BearerTokenErrorCode: %v", beErr.ErrorCode) + ctx.ServerError("Unhandled BearerTokenError", fmt.Errorf("BearerTokenError: error=\"%v\", error_description=\"%v\"", beErr.ErrorCode, beErr.ErrorDescription)) } }