Skip to content

Commit

Permalink
Merge remote-tracking branch 'giteaofficial/main'
Browse files Browse the repository at this point in the history
* giteaofficial/main:
  Fix incorrect `/tokens` api (go-gitea#32085)
  Set manual `tabindex`es on login page (go-gitea#31689)
  Only use Host header from reverse proxy (go-gitea#32060)
  [skip ci] Updated translations via Crowdin
  • Loading branch information
zjjhot committed Sep 21, 2024
2 parents 3cd37d1 + 08adbc4 commit 19aa38d
Show file tree
Hide file tree
Showing 8 changed files with 32 additions and 39 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/pull-db-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,8 @@ jobs:
runs-on: ubuntu-latest
services:
mssql:
image: mcr.microsoft.com/mssql/server:2017-latest
# some images before 2024-04 can't run on new kernels
image: mcr.microsoft.com/mssql/server:2019-latest
env:
ACCEPT_EULA: Y
MSSQL_PID: Standard
Expand Down
13 changes: 3 additions & 10 deletions modules/httplib/url.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,6 @@ func getRequestScheme(req *http.Request) string {
return ""
}

func getForwardedHost(req *http.Request) string {
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Host
return req.Header.Get("X-Forwarded-Host")
}

// GuessCurrentAppURL tries to guess the current full app URL (with sub-path) by http headers. It always has a '/' suffix, exactly the same as setting.AppURL
func GuessCurrentAppURL(ctx context.Context) string {
return GuessCurrentHostURL(ctx) + setting.AppSubURL + "/"
Expand All @@ -81,11 +76,9 @@ func GuessCurrentHostURL(ctx context.Context) string {
if reqScheme == "" {
return strings.TrimSuffix(setting.AppURL, setting.AppSubURL+"/")
}
reqHost := getForwardedHost(req)
if reqHost == "" {
reqHost = req.Host
}
return reqScheme + "://" + reqHost
// X-Forwarded-Host has many problems: non-standard, not well-defined (X-Forwarded-Port or not), conflicts with Host header.
// So do not use X-Forwarded-Host, just use Host header directly.
return reqScheme + "://" + req.Host
}

// MakeAbsoluteURL tries to make a link to an absolute URL:
Expand Down
5 changes: 3 additions & 2 deletions modules/httplib/url_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func TestMakeAbsoluteURL(t *testing.T) {
"X-Forwarded-Proto": {"https"},
},
})
assert.Equal(t, "https://forwarded-host/foo", MakeAbsoluteURL(ctx, "/foo"))
assert.Equal(t, "https://user-host/foo", MakeAbsoluteURL(ctx, "/foo"))
}

func TestIsCurrentGiteaSiteURL(t *testing.T) {
Expand Down Expand Up @@ -119,5 +119,6 @@ func TestIsCurrentGiteaSiteURL(t *testing.T) {
},
})
assert.True(t, IsCurrentGiteaSiteURL(ctx, "http://localhost:3000"))
assert.True(t, IsCurrentGiteaSiteURL(ctx, "https://forwarded-host"))
assert.True(t, IsCurrentGiteaSiteURL(ctx, "https://user-host"))
assert.False(t, IsCurrentGiteaSiteURL(ctx, "https://forwarded-host"))
}
1 change: 1 addition & 0 deletions options/locale/locale_pt-PT.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1731,6 +1731,7 @@ issues.dependency.add_error_dep_not_same_repo=Ambas as questões têm que estar
issues.review.self.approval=Não pode aprovar o seu próprio pedido de integração.
issues.review.self.rejection=Não pode solicitar modificações sobre o seu próprio pedido de integração.
issues.review.approve=aprovou estas modificações %s
issues.review.comment=reviu %s
issues.review.dismissed=descartou a revisão de %s %s
issues.review.dismissed_label=Descartada
issues.review.left_comment=deixou um comentário
Expand Down
5 changes: 5 additions & 0 deletions routers/api/v1/user/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,10 @@ func CreateAccessToken(ctx *context.APIContext) {
ctx.Error(http.StatusBadRequest, "AccessTokenScope.Normalize", fmt.Errorf("invalid access token scope provided: %w", err))
return
}
if scope == "" {
ctx.Error(http.StatusBadRequest, "AccessTokenScope", "access token must have a scope")
return
}
t.Scope = scope

if err := auth_model.NewAccessToken(ctx, t); err != nil {
Expand All @@ -129,6 +133,7 @@ func CreateAccessToken(ctx *context.APIContext) {
Token: t.Token,
ID: t.ID,
TokenLastEight: t.TokenLastEight,
Scopes: t.Scope.StringSlice(),
})
}

Expand Down
12 changes: 7 additions & 5 deletions templates/user/auth/signin_inner.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -14,28 +14,30 @@
{{.CsrfTokenHtml}}
<div class="required field {{if and (.Err_UserName) (or (not .LinkAccountMode) (and .LinkAccountMode .LinkAccountModeSignIn))}}error{{end}}">
<label for="user_name">{{ctx.Locale.Tr "home.uname_holder"}}</label>
<input id="user_name" type="text" name="user_name" value="{{.user_name}}" autofocus required>
<input id="user_name" type="text" name="user_name" value="{{.user_name}}" autofocus required tabindex="1">
</div>
{{if or (not .DisablePassword) .LinkAccountMode}}
<div class="required field {{if and (.Err_Password) (or (not .LinkAccountMode) (and .LinkAccountMode .LinkAccountModeSignIn))}}error{{end}} form-field-content-aside-label">
<label for="password">{{ctx.Locale.Tr "password"}}</label>
<a href="{{AppSubUrl}}/user/forgot_password">{{ctx.Locale.Tr "auth.forgot_password"}}</a>
<input id="password" name="password" type="password" value="{{.password}}" autocomplete="current-password" required>
<div>
<a href="{{AppSubUrl}}/user/forgot_password" tabindex="4">{{ctx.Locale.Tr "auth.forgot_password"}}</a>
</div>
<input id="password" name="password" type="password" value="{{.password}}" autocomplete="current-password" required tabindex="2">
</div>
{{end}}
{{if not .LinkAccountMode}}
<div class="inline field">
<div class="ui checkbox">
<label>{{ctx.Locale.Tr "auth.remember_me"}}</label>
<input name="remember" type="checkbox">
<input name="remember" type="checkbox" tabindex="5">
</div>
</div>
{{end}}

{{template "user/auth/captcha" .}}

<div class="field">
<button class="ui primary button tw-w-full">
<button class="ui primary button tw-w-full" tabindex="3">
{{if .LinkAccountMode}}
{{ctx.Locale.Tr "auth.oauth_signin_submit"}}
{{else}}
Expand Down
31 changes: 11 additions & 20 deletions tests/integration/api_token_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ func TestAPICreateAndDeleteToken(t *testing.T) {
defer tests.PrepareTestEnv(t)()
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})

newAccessToken := createAPIAccessTokenWithoutCleanUp(t, "test-key-1", user, nil)
newAccessToken := createAPIAccessTokenWithoutCleanUp(t, "test-key-1", user, []auth_model.AccessTokenScope{auth_model.AccessTokenScopeAll})
deleteAPIAccessToken(t, newAccessToken, user)

newAccessToken = createAPIAccessTokenWithoutCleanUp(t, "test-key-2", user, nil)
newAccessToken = createAPIAccessTokenWithoutCleanUp(t, "test-key-2", user, []auth_model.AccessTokenScope{auth_model.AccessTokenScopeAll})
deleteAPIAccessToken(t, newAccessToken, user)
}

Expand Down Expand Up @@ -72,19 +72,19 @@ func TestAPIDeleteTokensPermission(t *testing.T) {
user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})

// admin can delete tokens for other users
createAPIAccessTokenWithoutCleanUp(t, "test-key-1", user2, nil)
createAPIAccessTokenWithoutCleanUp(t, "test-key-1", user2, []auth_model.AccessTokenScope{auth_model.AccessTokenScopeAll})
req := NewRequest(t, "DELETE", "/api/v1/users/"+user2.LoginName+"/tokens/test-key-1").
AddBasicAuth(admin.Name)
MakeRequest(t, req, http.StatusNoContent)

// non-admin can delete tokens for himself
createAPIAccessTokenWithoutCleanUp(t, "test-key-2", user2, nil)
createAPIAccessTokenWithoutCleanUp(t, "test-key-2", user2, []auth_model.AccessTokenScope{auth_model.AccessTokenScopeAll})
req = NewRequest(t, "DELETE", "/api/v1/users/"+user2.LoginName+"/tokens/test-key-2").
AddBasicAuth(user2.Name)
MakeRequest(t, req, http.StatusNoContent)

// non-admin can't delete tokens for other users
createAPIAccessTokenWithoutCleanUp(t, "test-key-3", user2, nil)
createAPIAccessTokenWithoutCleanUp(t, "test-key-3", user2, []auth_model.AccessTokenScope{auth_model.AccessTokenScopeAll})
req = NewRequest(t, "DELETE", "/api/v1/users/"+user2.LoginName+"/tokens/test-key-3").
AddBasicAuth(user4.Name)
MakeRequest(t, req, http.StatusForbidden)
Expand Down Expand Up @@ -520,7 +520,7 @@ func runTestCase(t *testing.T, testCase *requiredScopeTestCase, user *user_model
unauthorizedScopes = append(unauthorizedScopes, cateogoryUnauthorizedScopes...)
}

accessToken := createAPIAccessTokenWithoutCleanUp(t, "test-token", user, &unauthorizedScopes)
accessToken := createAPIAccessTokenWithoutCleanUp(t, "test-token", user, unauthorizedScopes)
defer deleteAPIAccessToken(t, accessToken, user)

// Request the endpoint. Verify that permission is denied.
Expand All @@ -532,20 +532,12 @@ func runTestCase(t *testing.T, testCase *requiredScopeTestCase, user *user_model

// createAPIAccessTokenWithoutCleanUp Create an API access token and assert that
// creation succeeded. The caller is responsible for deleting the token.
func createAPIAccessTokenWithoutCleanUp(t *testing.T, tokenName string, user *user_model.User, scopes *[]auth_model.AccessTokenScope) api.AccessToken {
func createAPIAccessTokenWithoutCleanUp(t *testing.T, tokenName string, user *user_model.User, scopes []auth_model.AccessTokenScope) api.AccessToken {
payload := map[string]any{
"name": tokenName,
}
if scopes != nil {
for _, scope := range *scopes {
scopes, scopesExists := payload["scopes"].([]string)
if !scopesExists {
scopes = make([]string, 0)
}
scopes = append(scopes, string(scope))
payload["scopes"] = scopes
}
"name": tokenName,
"scopes": scopes,
}

log.Debug("Requesting creation of token with scopes: %v", scopes)
req := NewRequestWithJSON(t, "POST", "/api/v1/users/"+user.LoginName+"/tokens", payload).
AddBasicAuth(user.Name)
Expand All @@ -563,8 +555,7 @@ func createAPIAccessTokenWithoutCleanUp(t *testing.T, tokenName string, user *us
return newAccessToken
}

// createAPIAccessTokenWithoutCleanUp Delete an API access token and assert that
// deletion succeeded.
// deleteAPIAccessToken deletes an API access token and assert that deletion succeeded.
func deleteAPIAccessToken(t *testing.T, accessToken api.AccessToken, user *user_model.User) {
req := NewRequestf(t, "DELETE", "/api/v1/users/"+user.LoginName+"/tokens/%d", accessToken.ID).
AddBasicAuth(user.Name)
Expand Down
1 change: 0 additions & 1 deletion web_src/css/form.css
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,6 @@ textarea:focus,
}
.form-field-content-aside-label > *:nth-child(2) {
text-align: right;
margin-bottom: 4px;
}
.form-field-content-aside-label input {
grid-column: span 2;
Expand Down

0 comments on commit 19aa38d

Please sign in to comment.