Skip to content
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

Jump to bottom

Show OAuth2 errors to end users #25261

Merged
merged 3 commits into from
Jun 15, 2023
Merged

Show OAuth2 errors to end users #25261

Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion options/locale/locale_en-US.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2901,7 +2901,7 @@ auths.sspi_default_language = Default user language
auths.sspi_default_language_helper = Default language for users automatically created by SSPI auth method. Leave empty if you prefer language to be automatically detected.
auths.tips = Tips
auths.tips.oauth2.general = OAuth2 Authentication
auths.tips.oauth2.general.tip = When registering a new OAuth2 authentication, the callback/redirect URL should be: <host>/user/oauth2/<Authentication Name>/callback
auths.tips.oauth2.general.tip = When registering a new OAuth2 authentication, the callback/redirect URL should be:
auths.tip.oauth2_provider = OAuth2 Provider
auths.tip.bitbucket = Register a new OAuth consumer on https://bitbucket.org/account/user/<your username>/oauth-consumers/new and add the permission 'Account' - 'Read'
auths.tip.nextcloud = Register a new OAuth consumer on your instance using the following menu "Settings -> Security -> OAuth 2.0 client"
Expand Down
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
15 changes: 13 additions & 2 deletions templates/admin/auth/edit.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
<span>{{.Source.TypeName}}</span>
</div>
<div class="required inline field {{if .Err_Name}}error{{end}}">
<label for="name">{{.locale.Tr "admin.auths.auth_name"}}</label>
<input id="name" name="name" value="{{.Source.Name}}" autofocus required>
<label for="auth_name">{{.locale.Tr "admin.auths.auth_name"}}</label>
<input id="auth_name" name="name" value="{{.Source.Name}}" autofocus required>
</div>

<!-- LDAP and DLDAP -->
Expand Down 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 class="oauth2">{{.locale.Tr "admin.auths.tips.oauth2.general"}}:</h5>
<p class="oauth2">{{.locale.Tr "admin.auths.tips.oauth2.general.tip"}} <b id="oauth2-callback-url"></b></p>
</div>
Copy link
Member

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

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
8 changes: 4 additions & 4 deletions templates/admin/auth/new.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
</div>
</div>
<div class="required inline field {{if .Err_Name}}error{{end}}">
<label for="name">{{.locale.Tr "admin.auths.auth_name"}}</label>
<input id="name" name="name" value="{{.name}}" autofocus required>
<label for="auth_name">{{.locale.Tr "admin.auths.auth_name"}}</label>
<input id="auth_name" name="name" value="{{.name}}" autofocus required>
</div>

<!-- LDAP and DLDAP -->
Expand Down Expand Up @@ -85,8 +85,8 @@
<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>
<h5 class="oauth2">{{.locale.Tr "admin.auths.tips.oauth2.general"}}:</h5>
<p class="oauth2">{{.locale.Tr "admin.auths.tips.oauth2.general.tip"}} <b id="oauth2-callback-url"></b></p>

<h5 class="ui top attached header">{{.locale.Tr "admin.auths.tip.oauth2_provider"}}</h5>
<div class="ui attached segment">
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
6 changes: 6 additions & 0 deletions web_src/js/features/admin/common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,12 @@ export function initAdminCommon() {
}
}

if ($('.admin.authentication').length > 0) {
$('#auth_name').on('input', function () {
$('#oauth2-callback-url').text(`${window.location.origin}/user/oauth2/${encodeURIComponent($(this).val())}/callback`);
}).trigger('input');
}

// Notice
if ($('.admin.notice')) {
const $detailModal = $('#detail-modal');
Expand Down