Skip to content

Show OAuth2 errors to end users #25261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lunny merged 3 commits into from
Jun 15, 2023
Merged

Show OAuth2 errors to end users #25261

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8e9f5e1
fix
wxiaoguang Jun 14, 2023
8afc31b
show real callback URL for auth source
wxiaoguang Jun 14, 2023
6d248b9
Merge branch 'main' into show-oauth2-err
GiteaBot Jun 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions routers/web/auth/oauth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@
package auth

import (
stdContext "context"
go_context "context"
"encoding/base64"
"errors"
"fmt"
"html"
"io"
"net/http"
"net/url"
"sort"
"strings"

"code.gitea.io/gitea/models/auth"
Expand Down Expand Up @@ -39,6 +40,7 @@ import (
"github.com/golang-jwt/jwt/v4"
"github.com/markbates/goth"
"github.com/markbates/goth/gothic"
go_oauth2 "golang.org/x/oauth2"
)

const (
Expand Down Expand Up @@ -143,7 +145,7 @@ type AccessTokenResponse struct {
IDToken string `json:"id_token,omitempty"`
}

func newAccessTokenResponse(ctx stdContext.Context, grant *auth.OAuth2Grant, serverKey, clientKey oauth2.JWTSigningKey) (*AccessTokenResponse, *AccessTokenError) {
func newAccessTokenResponse(ctx go_context.Context, grant *auth.OAuth2Grant, serverKey, clientKey oauth2.JWTSigningKey) (*AccessTokenResponse, *AccessTokenError) {
if setting.OAuth2.InvalidateRefreshTokens {
if err := grant.IncreaseCounter(ctx); err != nil {
return nil, &AccessTokenError{
Expand Down Expand Up @@ -886,6 +888,17 @@ func SignInOAuth(ctx *context.Context) {
func SignInOAuthCallback(ctx *context.Context) {
provider := ctx.Params(":provider")

if ctx.Req.FormValue("error") != "" {
var errorKeyValues []string
for k, vv := range ctx.Req.Form {
for _, v := range vv {
errorKeyValues = append(errorKeyValues, fmt.Sprintf("%s = %s", html.EscapeString(k), html.EscapeString(v)))
}
}
sort.Strings(errorKeyValues)
ctx.Flash.Error(strings.Join(errorKeyValues, "<br>"), true)
}

// first look if the provider is still active
authSource, err := auth.GetActiveOAuth2SourceByName(provider)
if err != nil {
Expand All @@ -894,7 +907,7 @@ func SignInOAuthCallback(ctx *context.Context) {
}

if authSource == nil {
ctx.ServerError("SignIn", errors.New("No valid provider found, check configured callback url in provider"))
ctx.ServerError("SignIn", errors.New("no valid provider found, check configured callback url in provider"))
return
}

Expand All @@ -920,6 +933,9 @@ func SignInOAuthCallback(ctx *context.Context) {
ctx.Redirect(setting.AppSubURL + "/user/login")
return
}
if err, ok := err.(*go_oauth2.RetrieveError); ok {
ctx.Flash.Error("OAuth2 RetrieveError: "+err.Error(), true)
}
ctx.ServerError("UserSignIn", err)
return
}
Expand Down
11 changes: 11 additions & 0 deletions templates/admin/auth/edit.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,17 @@
</div>
</form>
</div>

<h4 class="ui top attached header">
{{.locale.Tr "admin.auths.tips"}}
</h4>
<div class="ui attached segment">
<h5>GMail Settings:</h5>
<p>Host: smtp.gmail.com, Port: 587, Enable TLS Encryption: true</p>

<h5>{{.locale.Tr "admin.auths.tips.oauth2.general"}}:</h5>
<p>{{.locale.Tr "admin.auths.tips.oauth2.general.tip"}}</p>
</div>
Copy link
Member

@silverwind silverwind Jun 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot of this?

Copy link
Contributor Author

@wxiaoguang wxiaoguang Jun 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just copied from the "new" page. Nothing special.

image

</div>

<div class="ui g-modal-confirm delete modal">
Expand Down
5 changes: 5 additions & 0 deletions templates/status/500.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{{/* This page should only depend the minimal template functions/variables, to avoid triggering new panics.
* base template functions: AppName, AssetUrlPrefix, AssetVersion, AppSubUrl, DefaultTheme, Str2html
* locale
* Flash
* ErrorMsg
* SignedUser (optional)
*/}}
Expand Down Expand Up @@ -28,6 +29,10 @@
</div>
</nav>
<div role="main" class="page-content status-page-500">
<div class="ui container" >
<style> .ui.message.flash-message { text-align: left; } </style>
{{template "base/alert" .}}
</div>
<p class="gt-mt-5 center"><img src="{{AssetUrlPrefix}}/img/500.png" alt="Internal Server Error"></p>
<div class="ui divider"></div>
<div class="ui container gt-my-5">
Expand Down