diff --git a/models/organization/org_test.go b/models/organization/org_test.go index 7a74c5f5fc..be27dbb96c 100644 --- a/models/organization/org_test.go +++ b/models/organization/org_test.go @@ -456,6 +456,22 @@ func TestGetUsersWhoCanCreateOrgRepo(t *testing.T) { assert.NotNil(t, users[5]) } +func TestCanCreateOrgRepoByOwnerTeamWithoutFlag(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + org := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3}) + ownerTeam, err := org.GetOwnerTeam(t.Context()) + require.NoError(t, err) + + ownerTeam.CanCreateOrgRepo = false + _, err = db.GetEngine(t.Context()).ID(ownerTeam.ID).Cols("can_create_org_repo").Update(ownerTeam) + require.NoError(t, err) + + ok, err := organization.CanCreateOrgRepo(t.Context(), org.ID, 2) + require.NoError(t, err) + assert.True(t, ok) +} + func TestUser_RemoveOrgRepo(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) org := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3}) diff --git a/models/organization/org_user.go b/models/organization/org_user.go index 627c1c2edf..7d0476606c 100644 --- a/models/organization/org_user.go +++ b/models/organization/org_user.go @@ -8,6 +8,7 @@ import ( "fmt" "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/perm" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/log" @@ -104,7 +105,7 @@ func IsPublicMembership(ctx context.Context, orgID, uid int64) (bool, error) { // CanCreateOrgRepo returns true if user can create repo in organization func CanCreateOrgRepo(ctx context.Context, orgID, uid int64) (bool, error) { return db.GetEngine(ctx). - Where(builder.Eq{"team.can_create_org_repo": true}). + Where(builder.Eq{"team.can_create_org_repo": true}.Or(builder.Eq{"team.authorize": perm.AccessModeOwner})). Join("INNER", "team_user", "team_user.team_id = team.id"). And("team_user.uid = ?", uid). And("team_user.org_id = ?", orgID). diff --git a/renovate.json5 b/renovate.json5 index 382b837820..c5d712dc0a 100644 --- a/renovate.json5 +++ b/renovate.json5 @@ -2,7 +2,7 @@ "$schema": "https://docs.renovatebot.com/renovate-schema.json", "extends": ["config:recommended", "helpers:pinGitHubActionDigests", "customManagers:githubActionsVersions"], "configMigration": true, - "enabledManagers": ["github-actions", "gomod", "npm", "pep621", "nix", "custom.regex"], + "enabledManagers": ["github-actions", "gomod", "npm", "pep621", "nix", "custom.regex", "dockerfile"], "labels": ["dependencies"], "branchPrefix": "renovate/", "schedule": ["* * * * 1"], // dependency update PRs weekly, vulnerabilityAlerts bypasses this @@ -115,5 +115,9 @@ "groupName": "nix dependencies", "matchManagers": ["nix"], }, + { + "groupName": "docker dependencies", + "matchManagers": ["dockerfile"], + }, ], } diff --git a/routers/web/auth/oauth2_provider.go b/routers/web/auth/oauth2_provider.go index 05931d8f59..b4f36d543c 100644 --- a/routers/web/auth/oauth2_provider.go +++ b/routers/web/auth/oauth2_provider.go @@ -561,6 +561,13 @@ func handleRefreshToken(ctx *context.Context, form forms.AccessTokenForm, server }) return } + if grant.ApplicationID != app.ID { + handleAccessTokenError(ctx, oauth2_provider.AccessTokenError{ + ErrorCode: oauth2_provider.AccessTokenErrorCodeInvalidGrant, + ErrorDescription: "refresh token belongs to a different client", + }) + return + } // check if token got already used if setting.OAuth2.InvalidateRefreshTokens && (grant.Counter != token.Counter || token.Counter == 0) { @@ -630,6 +637,13 @@ func handleAuthorizationCode(ctx *context.Context, form forms.AccessTokenForm, s }) return } + if authorizationCode.RedirectURI != "" && form.RedirectURI != authorizationCode.RedirectURI { + handleAccessTokenError(ctx, oauth2_provider.AccessTokenError{ + ErrorCode: oauth2_provider.AccessTokenErrorCodeInvalidGrant, + ErrorDescription: "redirect_uri differs from the original authorization request", + }) + return + } // check if granted for this application if authorizationCode.Grant.ApplicationID != app.ID { handleAccessTokenError(ctx, oauth2_provider.AccessTokenError{ diff --git a/services/doctor/fix8312.go b/services/doctor/fix8312.go deleted file mode 100644 index 3e2ca68eb4..0000000000 --- a/services/doctor/fix8312.go +++ /dev/null @@ -1,61 +0,0 @@ -// Copyright 2023 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package doctor - -import ( - "context" - - "code.gitea.io/gitea/models/db" - org_model "code.gitea.io/gitea/models/organization" - "code.gitea.io/gitea/models/perm" - "code.gitea.io/gitea/modules/log" - org_service "code.gitea.io/gitea/services/org" - - "xorm.io/builder" -) - -func fixOwnerTeamCreateOrgRepo(ctx context.Context, logger log.Logger, autofix bool) error { - count := 0 - - err := db.Iterate( - ctx, - builder.Eq{"authorize": perm.AccessModeOwner, "can_create_org_repo": false}, - func(ctx context.Context, team *org_model.Team) error { - team.CanCreateOrgRepo = true - count++ - - if !autofix { - return nil - } - - return org_service.UpdateTeam(ctx, team, false, false) - }, - ) - if err != nil { - logger.Critical("Unable to iterate across repounits to fix incorrect can_create_org_repo: Error %v", err) - return err - } - - if !autofix { - if count == 0 { - logger.Info("Found no team with incorrect can_create_org_repo") - } else { - logger.Warn("Found %d teams with incorrect can_create_org_repo", count) - } - return nil - } - logger.Info("Fixed %d teams with incorrect can_create_org_repo", count) - - return nil -} - -func init() { - Register(&Check{ - Title: "Check for incorrect can_create_org_repo for org owner teams", - Name: "fix-owner-team-create-org-repo", - IsDefault: false, - Run: fixOwnerTeamCreateOrgRepo, - Priority: 7, - }) -} diff --git a/tests/integration/oauth_test.go b/tests/integration/oauth_test.go index 90ff785ac1..cef2e96441 100644 --- a/tests/integration/oauth_test.go +++ b/tests/integration/oauth_test.go @@ -5,6 +5,7 @@ package integration import ( "bytes" + "crypto/sha256" "encoding/base64" "fmt" "image" @@ -58,6 +59,46 @@ func testOAuth2PrepareTestCode(t *testing.T) { require.NoError(t, err) } +func createOAuthTestApplication(t *testing.T, userName, name string, redirectURIs []string) *api.OAuth2Application { + t.Helper() + req := NewRequestWithJSON(t, "POST", "/api/v1/user/applications/oauth2", &api.CreateOAuth2ApplicationOptions{ + Name: name, + RedirectURIs: redirectURIs, + ConfidentialClient: true, + }).AddBasicAuth(userName) + resp := MakeRequest(t, req, http.StatusCreated) + created := DecodeJSON(t, resp, &api.OAuth2Application{}) + require.NotEmpty(t, created.ClientID) + require.NotEmpty(t, created.ClientSecret) + return created +} + +func issueOAuthAuthorizationCode(t *testing.T, user *user_model.User, app *api.OAuth2Application, redirectURI, scope string) (string, string) { + t.Helper() + + grant := &auth_model.OAuth2Grant{ + ApplicationID: app.ID, + UserID: user.ID, + Scope: scope, + } + require.NoError(t, db.Insert(t.Context(), grant)) + + verifier := "phase3-verifier-" + util.FastCryptoRandomHex(12) + challengeBytes := sha256.Sum256([]byte(verifier)) + code := "phase3-code-" + util.FastCryptoRandomHex(10) + + require.NoError(t, db.Insert(t.Context(), &auth_model.OAuth2AuthorizationCode{ + GrantID: grant.ID, + Code: code, + CodeChallenge: base64.RawURLEncoding.EncodeToString(challengeBytes[:]), + CodeChallengeMethod: "S256", + RedirectURI: redirectURI, + ValidUntil: timeutil.TimeStampNow() + 86400, + })) + + return code, verifier +} + func TestOAuth2(t *testing.T) { defer tests.PrepareTestEnv(t)() @@ -72,12 +113,14 @@ func TestOAuth2(t *testing.T) { t.Run("AuthorizeRedirectWithExistingGrant", testAuthorizeRedirectWithExistingGrant) t.Run("AuthorizePKCERequiredForPublicClient", testAuthorizePKCERequiredForPublicClient) t.Run("AccessTokenExchange", testAccessTokenExchange) + t.Run("AccessTokenExchangeRedirectURIMismatch", testAccessTokenExchangeRedirectURIMismatch) t.Run("AccessTokenExchangeWithPublicClient", testAccessTokenExchangeWithPublicClient) t.Run("AccessTokenExchangeJSON", testAccessTokenExchangeJSON) t.Run("AccessTokenExchangeWithoutPKCE", testAccessTokenExchangeWithoutPKCE) t.Run("AccessTokenExchangeWithInvalidCredentials", testAccessTokenExchangeWithInvalidCredentials) t.Run("AccessTokenExchangeWithBasicAuth", testAccessTokenExchangeWithBasicAuth) t.Run("RefreshTokenInvalidation", testRefreshTokenInvalidation) + t.Run("RefreshTokenCrossClientUsage", testRefreshTokenCrossClientUsage) t.Run("OAuthIntrospection", testOAuthIntrospection) t.Run("OAuthGrantScopesReadUserFailRepos", testOAuthGrantScopesReadUserFailRepos) t.Run("OAuthGrantScopesBasicRespectsWriteUser", testOAuthGrantScopesBasicRespectsWriteUser) @@ -225,12 +268,43 @@ func testAccessTokenExchange(t *testing.T) { assert.Greater(t, len(parsed.RefreshToken), 10) } +func testAccessTokenExchangeRedirectURIMismatch(t *testing.T) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + redirectURIs := []string{"https://phase3.example/callback", "https://phase3.example/callback-alt"} + app := createOAuthTestApplication(t, user.Name, "phase3-redirect-uri-guard", redirectURIs) + code, verifier := issueOAuthAuthorizationCode(t, user, app, redirectURIs[0], "openid profile") + + req := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "authorization_code", + "client_id": app.ClientID, + "client_secret": app.ClientSecret, + "redirect_uri": redirectURIs[1], + "code": code, + "code_verifier": verifier, + }) + resp := MakeRequest(t, req, http.StatusBadRequest) + parsedError := new(oauth2_provider.AccessTokenError) + assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsedError)) + assert.Equal(t, "invalid_grant", string(parsedError.ErrorCode)) + assert.Equal(t, "redirect_uri differs from the original authorization request", parsedError.ErrorDescription) + + req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "authorization_code", + "client_id": app.ClientID, + "client_secret": app.ClientSecret, + "redirect_uri": redirectURIs[0], + "code": code, + "code_verifier": verifier, + }) + MakeRequest(t, req, http.StatusOK) +} + func testAccessTokenExchangeWithPublicClient(t *testing.T) { testOAuth2PrepareTestCode(t) req := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ "grant_type": "authorization_code", "client_id": "ce5a1322-42a7-11ed-b878-0242ac120002", - "redirect_uri": "http://127.0.0.1", + "redirect_uri": "http://127.0.0.1/", "code": "authcodepublic", "code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt", }) @@ -525,6 +599,54 @@ func testRefreshTokenInvalidation(t *testing.T) { assert.Equal(t, "token was already used", parsedError.ErrorDescription) } +func testRefreshTokenCrossClientUsage(t *testing.T) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + primaryApp := createOAuthTestApplication(t, user.Name, "phase3-refresh-token-primary", []string{"https://phase3.example/refresh-primary"}) + secondaryApp := createOAuthTestApplication(t, user.Name, "refresh-token-client-guard", []string{"https://alt-client.example/oauth/callback"}) + code, verifier := issueOAuthAuthorizationCode(t, user, primaryApp, primaryApp.RedirectURIs[0], "openid profile") + + req := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "authorization_code", + "client_id": primaryApp.ClientID, + "client_secret": primaryApp.ClientSecret, + "redirect_uri": primaryApp.RedirectURIs[0], + "code": code, + "code_verifier": verifier, + }) + resp := MakeRequest(t, req, http.StatusOK) + type response struct { + AccessToken string `json:"access_token"` + TokenType string `json:"token_type"` + ExpiresIn int64 `json:"expires_in"` + RefreshToken string `json:"refresh_token"` + } + parsed := new(response) + assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsed)) + assert.NotEmpty(t, parsed.RefreshToken) + + req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "refresh_token", + "client_id": secondaryApp.ClientID, + "client_secret": secondaryApp.ClientSecret, + "redirect_uri": secondaryApp.RedirectURIs[0], + "refresh_token": parsed.RefreshToken, + }) + resp = MakeRequest(t, req, http.StatusBadRequest) + parsedError := new(oauth2_provider.AccessTokenError) + assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsedError)) + assert.Equal(t, "invalid_grant", string(parsedError.ErrorCode)) + assert.Equal(t, "refresh token belongs to a different client", parsedError.ErrorDescription) + + req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "refresh_token", + "client_id": primaryApp.ClientID, + "client_secret": primaryApp.ClientSecret, + "redirect_uri": primaryApp.RedirectURIs[0], + "refresh_token": parsed.RefreshToken, + }) + MakeRequest(t, req, http.StatusOK) +} + func testOAuthIntrospection(t *testing.T) { testOAuth2PrepareTestCode(t) req := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{