Feature/oauth userinfo #15721

lunny · 2021-05-04T17:55:27Z

NLH-Software commented

2021-05-04 17:55:27 +00:00

(Migrated from github.com)

PullRequest which adds the userinfo endpoint for openid-connect.
Solves #8534 (https://github.com/go-gitea/gitea/issues/8534)
Obsoletes the simple solution in pull request #14938.
Can be used with grafana, mod_auth_openidc (tested on arm64/amd64)

Example-Configuration for mod_auth_openidc:
OIDCProviderMetadataURL <host/path_to>/.well-known/openid-configuration
OIDCClientID <clientid from gitea>
OIDCClientSecret <clientsecret from gitea>
OIDCProviderTokenEndpointParams client_secret=<clientsecret from gitea urlencoded>&client_id=<clientid from gitea urlencoded>
OIDCCryptoPassphrase <any random string>
OIDCRedirectURI <uri back to a non-existent path inside the protected url>

PullRequest which adds the userinfo endpoint for openid-connect. Solves #8534 (https://github.com/go-gitea/gitea/issues/8534) Obsoletes the simple solution in pull request #14938. Can be used with grafana, mod_auth_openidc (tested on arm64/amd64) Example-Configuration for mod_auth_openidc: OIDCProviderMetadataURL <host/path_to>/.well-known/openid-configuration OIDCClientID <clientid from gitea> OIDCClientSecret <clientsecret from gitea> OIDCProviderTokenEndpointParams client_secret=<clientsecret from gitea urlencoded>&client_id=<clientid from gitea urlencoded> OIDCCryptoPassphrase <any random string> OIDCRedirectURI <uri back to a non-existent path inside the protected url>

👍 6

lunny reviewed 2021-05-05 05:20:44 +00:00

routers/user/oauth.go

						
				@ -196,0 +243,4 @@

					}

					response := &userInfoResponse{

						Sub:      fmt.Sprint(authUser.ID),

						Name:     authUser.FullName,

lunny commented

2021-05-05 05:20:43 +00:00

if uid == 0, we should still give an error.

NLH-Software (Migrated from github.com) reviewed 2021-05-05 07:06:51 +00:00

routers/user/oauth.go

						
				@ -196,0 +243,4 @@

					}

					response := &userInfoResponse{

						Sub:      fmt.Sprint(authUser.ID),

						Name:     authUser.FullName,

NLH-Software (Migrated from github.com) commented

2021-05-05 07:06:50 +00:00

Good point, will add this afternoon.

lunny reviewed 2021-05-05 14:36:17 +00:00

routers/user/oauth.go

						
				@ -196,0 +243,4 @@

					}

					response := &userInfoResponse{

						Sub:      fmt.Sprint(authUser.ID),

						Name:     authUser.FullName,

lunny commented

2021-05-05 14:36:17 +00:00

If we can check uid == 0 and return, it's a better control flow on Golang.

And it should not be a server error but an invalid token.

If we can check `uid == 0` and return, it's a better control flow on Golang. And it should not be a server error but an invalid token.

NLH-Software (Migrated from github.com) reviewed 2021-05-05 15:34:51 +00:00

routers/user/oauth.go

						
				@ -196,0 +243,4 @@

					}

					response := &userInfoResponse{

						Sub:      fmt.Sprint(authUser.ID),

						Name:     authUser.FullName,

NLH-Software (Migrated from github.com) commented

2021-05-05 15:34:51 +00:00

Ah ok, so you mean a

if uid == 0 {
  handleAccessTokenError()
  return
}

before the
if uid != 0 {
?
Or should I remove the If statement uid != 0 before the next block?

Sry, but I'm new to golang, so I am not so firm with control flows on Golang. (Coming from Python and some other languages)

Ah ok, so you mean a ``` if uid == 0 { handleAccessTokenError() return } ``` before the `if uid != 0 {` ? Or should I remove the If statement uid != 0 before the next block? Sry, but I'm new to golang, so I am not so firm with control flows on Golang. (Coming from Python and some other languages)

NLH-Software (Migrated from github.com) reviewed 2021-05-05 15:45:54 +00:00

routers/user/oauth.go

						
				@ -196,0 +243,4 @@

					}

					response := &userInfoResponse{

						Sub:      fmt.Sprint(authUser.ID),

						Name:     authUser.FullName,

NLH-Software (Migrated from github.com) commented

2021-05-05 15:45:54 +00:00

Ok, just found RFC6750 for invalid bearer token usage.
Will implement a new handleBearerTokenError() function which should be used when there is an error at the UserInfo Endpoint according to the specs.

Ok, just found RFC6750 for invalid bearer token usage. Will implement a new handleBearerTokenError() function which should be used when there is an error at the UserInfo Endpoint according to the specs.

NLH-Software (Migrated from github.com) reviewed 2021-05-05 18:04:51 +00:00

routers/user/oauth.go

						
				@ -196,0 +243,4 @@

					}

					response := &userInfoResponse{

						Sub:      fmt.Sprint(authUser.ID),

						Name:     authUser.FullName,

NLH-Software (Migrated from github.com) commented

2021-05-05 18:04:50 +00:00

Implemented handleBearerTokenError and the BearerTokenError according to RFC 6750.
I didn't invalidate the token inside gitea, because the access token for itself is valid at the client and in RFC 6750 stands nothing about invalidation of any accesstoken, it should only respond with an invalid_token error and status code 401.

Implemented handleBearerTokenError and the BearerTokenError according to RFC 6750. I didn't invalidate the token inside gitea, because the access token for itself is valid at the client and in RFC 6750 stands nothing about invalidation of any accesstoken, it should only respond with an invalid_token error and status code 401.

techknowlogick reviewed 2021-05-06 00:28:28 +00:00

routers/user/oauth.go

techknowlogick commented

2021-05-06 00:28:28 +00:00

If execution reaches this point, then it can be assumed that uid is not 0 (since if it were, then the above return would be hit), so you don't need to wrap the below in an if

If execution reaches this point, then it can be assumed that uid is not 0 (since if it were, then the above `return` would be hit), so you don't need to wrap the below in an if

lunny reviewed 2021-05-06 00:46:33 +00:00

routers/user/oauth.go

						
				@ -571,3 +629,18 @@ func handleAuthorizeError(ctx *context.Context, authErr AuthorizeError, redirect

					redirect.RawQuery = q.Encode()

					ctx.Redirect(redirect.String(), 302)

lunny commented

2021-05-06 00:46:33 +00:00

Use switch is better

lunny approved these changes 2021-05-06 00:47:33 +00:00

lunny left a comment

LGTM except @techknowlogick 's comment

NLH-Software (Migrated from github.com) reviewed 2021-05-06 04:56:59 +00:00

routers/user/oauth.go

						
				@ -571,3 +629,18 @@ func handleAuthorizeError(ctx *context.Context, authErr AuthorizeError, redirect

					redirect.RawQuery = q.Encode()

					ctx.Redirect(redirect.String(), 302)

NLH-Software (Migrated from github.com) commented

2021-05-06 04:56:59 +00:00

Implemented switch (came from Python, there is no switch available - my fault..)
Added also a default for unknown ErrorCode (this would be a server error, because it should never be reached, only if there is some internal failure)

Implemented switch (came from Python, there is no switch available - my fault..) Added also a default for unknown ErrorCode (this would be a server error, because it should never be reached, only if there is some internal failure)

NLH-Software (Migrated from github.com) reviewed 2021-05-06 04:57:23 +00:00

routers/user/oauth.go

NLH-Software (Migrated from github.com) commented

2021-05-06 04:57:23 +00:00

Removed the unneeded if statement.

techknowlogick approved these changes 2021-05-06 05:01:32 +00:00

techknowlogick left a comment

Thank you so much for this PR :)

Hopefully you had a pleasant experience, and will make more in the future as we'd be happy to have them :) (if it wasn't a pleasant experience please do let us know and we can improve our process)

Thank you so much for this PR :) Hopefully you had a pleasant experience, and will make more in the future as we'd be happy to have them :) (if it wasn't a pleasant experience please do let us know and we can improve our process)

This repo is archived. You cannot comment on pull requests.

No reviewers