diff --git a/models/auth/oauth2.go b/models/auth/oauth2.go index 9acd9a46b0..9ef89cb624 100644 --- a/models/auth/oauth2.go +++ b/models/auth/oauth2.go @@ -6,6 +6,7 @@ package auth import ( "context" "crypto/sha256" + "crypto/subtle" "encoding/base32" "encoding/base64" "errors" @@ -151,9 +152,9 @@ func (app *OAuth2Application) ContainsRedirectURI(redirectURI string) bool { // https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest // https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics-12#section-3.1 contains := func(s string) bool { - s = strings.TrimSuffix(strings.ToLower(s), "/") + s = strings.TrimSuffix(util.ToUpperASCII(s), "/") for _, u := range app.RedirectURIs { - if strings.TrimSuffix(strings.ToLower(u), "/") == s { + if strings.TrimSuffix(util.ToUpperASCII(u), "/") == s { return true } } @@ -408,26 +409,41 @@ func (code *OAuth2AuthorizationCode) GenerateRedirectURI(state string) (*url.URL return redirect, err } -// Invalidate deletes the auth code from the database to invalidate this code +// Invalidate deletes the auth code from the database to invalidate this code. +// It returns an error if the code was already invalidated (i.e., no rows were deleted), +// which prevents authorization code replay attacks. func (code *OAuth2AuthorizationCode) Invalidate(ctx context.Context) error { - _, err := db.GetEngine(ctx).ID(code.ID).NoAutoCondition().Delete(code) - return err + affected, err := db.GetEngine(ctx).ID(code.ID).NoAutoCondition().Delete(code) + if err != nil { + return err + } + if affected == 0 { + return fmt.Errorf("authorization code already used or does not exist") + } + return nil } -// ValidateCodeChallenge validates the given verifier against the saved code challenge. This is part of the PKCE implementation. +// ValidateCodeChallenge validates the given verifier against the saved code challenge. This is part of the PKCE +// implementation. If a code challenge was set during authorization, a valid verifier MUST be provided. func (code *OAuth2AuthorizationCode) ValidateCodeChallenge(verifier string) bool { + // If no PKCE was used during authorization, no verifier is needed. + if code.CodeChallengeMethod == "" && code.CodeChallenge == "" { + return true + } + // A challenge was set but no verifier provided: reject outright, no comparison or hashing is required. + if verifier == "" { + return false + } switch code.CodeChallengeMethod { case "S256": // base64url(SHA256(verifier)) see https://tools.ietf.org/html/rfc7636#section-4.6 h := sha256.Sum256([]byte(verifier)) hashedVerifier := base64.RawURLEncoding.EncodeToString(h[:]) - return hashedVerifier == code.CodeChallenge + return subtle.ConstantTimeCompare([]byte(hashedVerifier), []byte(code.CodeChallenge)) == 1 case "plain": - return verifier == code.CodeChallenge - case "": - return true + return subtle.ConstantTimeCompare([]byte(verifier), []byte(code.CodeChallenge)) == 1 default: - // unsupported method -> return false + // unsupported or empty method with a non-empty challenge -> reject return false } } @@ -490,16 +506,19 @@ func (grant *OAuth2Grant) GenerateNewAuthorizationCode(ctx context.Context, redi } // IncreaseCounter increases the counter and updates the grant +// IncreaseCounter atomically increments the counter only if it still matches +// the value loaded into this grant. Returns an error if the counter was already +// changed by a concurrent request (refresh token replay). func (grant *OAuth2Grant) IncreaseCounter(ctx context.Context) error { - _, err := db.GetEngine(ctx).ID(grant.ID).Incr("counter").Update(new(OAuth2Grant)) + affected, err := db.GetEngine(ctx).Where("id = ? AND counter = ?", grant.ID, grant.Counter). + Incr("counter").Update(new(OAuth2Grant)) if err != nil { return err } - updatedGrant, err := GetOAuth2GrantByID(ctx, grant.ID) - if err != nil { - return err + if affected == 0 { + return fmt.Errorf("grant counter changed unexpectedly (possible replay)") } - grant.Counter = updatedGrant.Counter + grant.Counter++ return nil } diff --git a/models/auth/oauth2_test.go b/models/auth/oauth2_test.go index 9c6836ed0d..aa474393c1 100644 --- a/models/auth/oauth2_test.go +++ b/models/auth/oauth2_test.go @@ -75,6 +75,13 @@ func TestOAuth2Application_ContainsRedirect_Slash(t *testing.T) { assert.False(t, app.ContainsRedirectURI("http://127.0.0.1/other")) } +func TestOAuth2Application_ContainsRedirect_Normalization(t *testing.T) { + app := &auth_model.OAuth2Application{RedirectURIs: []string{"https://website.com"}} + assert.True(t, app.ContainsRedirectURI("https://website.com")) + assert.True(t, app.ContainsRedirectURI("https://webSITE.com")) // ascii uppercase I + assert.False(t, app.ContainsRedirectURI("https://websİte.com")) // U+0130 as I, Latin Capital Letter I with Dot Above +} + func TestOAuth2Application_ValidateClientSecret(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) app := unittest.AssertExistsAndLoadBean(t, &auth_model.OAuth2Application{ID: 1}) @@ -147,9 +154,18 @@ func TestGetOAuth2GrantByID(t *testing.T) { func TestOAuth2Grant_IncreaseCounter(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) grant := unittest.AssertExistsAndLoadBean(t, &auth_model.OAuth2Grant{ID: 1, Counter: 1}) + + // First increment succeeds require.NoError(t, grant.IncreaseCounter(db.DefaultContext)) assert.Equal(t, int64(2), grant.Counter) unittest.AssertExistsAndLoadBean(t, &auth_model.OAuth2Grant{ID: 1, Counter: 2}) + + // Simulate a stale grant (counter still 1): must fail (concurrent replay) + grant.Counter = 1 + require.Error(t, grant.IncreaseCounter(db.DefaultContext), "stale counter must be rejected") + + // Counter in DB should be unchanged + unittest.AssertExistsAndLoadBean(t, &auth_model.OAuth2Grant{ID: 1, Counter: 2}) } func TestOAuth2Grant_ScopeContains(t *testing.T) { @@ -232,12 +248,33 @@ func TestOAuth2AuthorizationCode_ValidateCodeChallenge(t *testing.T) { } assert.False(t, code.ValidateCodeChallenge("foiwgjioriogeiogjerger")) - // test no code challenge + // test no PKCE at all (no challenge set, no verifier needed) + code = &auth_model.OAuth2AuthorizationCode{ + CodeChallengeMethod: "", + CodeChallenge: "", + } + assert.True(t, code.ValidateCodeChallenge("")) + + // test PKCE required: challenge was set but verifier is empty + code = &auth_model.OAuth2AuthorizationCode{ + CodeChallengeMethod: "S256", + CodeChallenge: "CjvyTLSdR47G5zYenDA-eDWW4lRrO8yvjcWwbD_deOg", + } + assert.False(t, code.ValidateCodeChallenge(""), "PKCE required: S256 challenge set but empty verifier must be rejected") + + code = &auth_model.OAuth2AuthorizationCode{ + CodeChallengeMethod: "plain", + CodeChallenge: "test123", + } + assert.False(t, code.ValidateCodeChallenge(""), "PKCE required: plain challenge set but empty verifier must be rejected") + + // test challenge stored but method empty (malformed: should reject) code = &auth_model.OAuth2AuthorizationCode{ CodeChallengeMethod: "", CodeChallenge: "foierjiogerogerg", } - assert.True(t, code.ValidateCodeChallenge("")) + assert.False(t, code.ValidateCodeChallenge(""), "challenge present with empty method must be rejected") + assert.False(t, code.ValidateCodeChallenge("foierjiogerogerg"), "challenge present with empty method must be rejected even with matching verifier") } func TestOAuth2AuthorizationCode_GenerateRedirectURI(t *testing.T) { @@ -262,6 +299,20 @@ func TestOAuth2AuthorizationCode_Invalidate(t *testing.T) { unittest.AssertNotExistsBean(t, &auth_model.OAuth2AuthorizationCode{Code: "authcode"}) } +func TestOAuth2AuthorizationCode_Invalidate_DoubleUse(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + code := unittest.AssertExistsAndLoadBean(t, &auth_model.OAuth2AuthorizationCode{Code: "authcode"}) + + // First invalidation should succeed + require.NoError(t, code.Invalidate(db.DefaultContext)) + unittest.AssertNotExistsBean(t, &auth_model.OAuth2AuthorizationCode{Code: "authcode"}) + + // Second invalidation of the same code must fail (replay prevention) + err := code.Invalidate(db.DefaultContext) + require.Error(t, err) + assert.Contains(t, err.Error(), "authorization code already used") +} + func TestOAuth2AuthorizationCode_TableName(t *testing.T) { assert.Equal(t, "oauth2_authorization_code", new(auth_model.OAuth2AuthorizationCode).TableName()) } diff --git a/release-notes/12494.md b/release-notes/12494.md new file mode 100644 index 0000000000..a26776b47c --- /dev/null +++ b/release-notes/12494.md @@ -0,0 +1,6 @@ +- fix: prevent git write to wiki repo from unauthorized user via git HTTP +- fix: prevent LFS authorization token from being used for read/write access after user's access is restricted from Forgejo +- fix: prevent scoped API access (OAuth tokens, Access tokens) from accessing resources beyond their permitted scope via non-API endpoints (e.g. /user/repo/raw/...) +- fix: implementing missing OAuth validation checks, improve protections against race conditions +- fix: prevent OAuth redirect URI spoofing via non-ascii case collision +- fix: strengthen Actions Artifact V4 signature algorithm against spoofing attacks diff --git a/release-notes/12495.md b/release-notes/12495.md new file mode 100644 index 0000000000..a26776b47c --- /dev/null +++ b/release-notes/12495.md @@ -0,0 +1,6 @@ +- fix: prevent git write to wiki repo from unauthorized user via git HTTP +- fix: prevent LFS authorization token from being used for read/write access after user's access is restricted from Forgejo +- fix: prevent scoped API access (OAuth tokens, Access tokens) from accessing resources beyond their permitted scope via non-API endpoints (e.g. /user/repo/raw/...) +- fix: implementing missing OAuth validation checks, improve protections against race conditions +- fix: prevent OAuth redirect URI spoofing via non-ascii case collision +- fix: strengthen Actions Artifact V4 signature algorithm against spoofing attacks diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index 56fe5cb2cc..c3d3c37e79 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -89,6 +89,7 @@ import ( "crypto/hmac" "crypto/sha256" "encoding/base64" + "encoding/binary" "encoding/xml" "fmt" "io" @@ -163,11 +164,20 @@ func ArtifactsV4Routes(prefix string) *web.Route { func (r artifactV4Routes) buildSignature(endp, expires, artifactName string, taskID, artifactID int64) []byte { mac := hmac.New(sha256.New, setting.GetGeneralTokenSigningSecret()) + // For each string field, write the length of the string first to prevent user-controlled strings from bleeding into + // each other and causing a signature match when it isn't expected. + _ = binary.Write(mac, binary.BigEndian, int64(len(endp))) mac.Write([]byte(endp)) + _ = binary.Write(mac, binary.BigEndian, int64(len(expires))) mac.Write([]byte(expires)) + _ = binary.Write(mac, binary.BigEndian, int64(len(artifactName))) mac.Write([]byte(artifactName)) - fmt.Fprint(mac, taskID) - fmt.Fprint(mac, artifactID) + + // Write TaskID & ArtifactID as binary values to prevent them from bleeding together; eg. TaskID="2" & + // ArtifactID="18" could look the same in the signature as TaskID="21" and ArtifactID="8" if they're written as + // strings. + _ = binary.Write(mac, binary.BigEndian, taskID) + _ = binary.Write(mac, binary.BigEndian, artifactID) return mac.Sum(nil) } diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index 914e181259..828027c0a8 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -803,6 +803,16 @@ func handleRefreshToken(ctx *context.Context, form forms.AccessTokenForm, server return } + // Ensure the refresh token's grant belongs to the requesting client. + // This prevents cross-client token usage (RFC 6749 Section 10.4). + if grant.ApplicationID != app.ID { + handleAccessTokenError(ctx, AccessTokenErrorResponse{ + ErrorCode: AccessTokenErrorCodeInvalidGrant, + ErrorDescription: "refresh token was not issued to this client", + }) + return + } + // check if token got already used if setting.OAuth2.InvalidateRefreshTokens && (grant.Counter != token.Counter || token.Counter == 0) { handleAccessTokenError(ctx, AccessTokenErrorResponse{ @@ -863,6 +873,15 @@ func handleAuthorizationCode(ctx *context.Context, form forms.AccessTokenForm, s }) return } + // Per RFC 6749 §4.1.3, if redirect_uri was included in the authorization request, + // it MUST be identical to the value included in the token request. + if authorizationCode.RedirectURI != "" && form.RedirectURI != authorizationCode.RedirectURI { + handleAccessTokenError(ctx, AccessTokenErrorResponse{ + ErrorCode: AccessTokenErrorCodeUnauthorizedClient, + ErrorDescription: "redirect_uri does not match the authorization request", + }) + return + } // check if granted for this application if authorizationCode.Grant.ApplicationID != app.ID { handleAccessTokenError(ctx, AccessTokenErrorResponse{ @@ -877,6 +896,7 @@ func handleAuthorizationCode(ctx *context.Context, form forms.AccessTokenForm, s ErrorCode: AccessTokenErrorCodeInvalidRequest, ErrorDescription: "cannot proceed your request", }) + return } resp, tokenErr := newAccessTokenResponse(ctx, authorizationCode.Grant, serverKey, clientKey) if tokenErr != nil { diff --git a/routers/web/repo/attachment.go b/routers/web/repo/attachment.go index 5497738f0b..65be51ba08 100644 --- a/routers/web/repo/attachment.go +++ b/routers/web/repo/attachment.go @@ -7,6 +7,7 @@ import ( "fmt" "net/http" + "forgejo.org/models/auth" access_model "forgejo.org/models/perm/access" repo_model "forgejo.org/models/repo" "forgejo.org/modules/httpcache" @@ -98,6 +99,30 @@ func ServeAttachment(ctx *context.Context, uuid string) { return } + // Some paths like `/attachment/{uuid}` can be accessed with API authentication mechanisms, which can have limited + // scopes (eg. user access tokens, OAuth grants). There's no middleware on these routes that enforce API scopes + // because (a) they're not API endpoints, and (b) the scoped required is dependant on the attachment. + if hasScope, scope := ctx.Authentication.Scope().Get(); hasScope { + var requiredScope auth.AccessTokenScope + if attach.ReleaseID != 0 { + requiredScope = auth.AccessTokenScopeReadRepository + } else if attach.IssueID != 0 { + requiredScope = auth.AccessTokenScopeReadIssue + } else { + ctx.ServerError("UnidentifiedAttachmentResource", fmt.Errorf("unable to identify resource for attachment id=%d", attach.ID)) + return + } + allow, err := scope.HasScope(requiredScope) + if err != nil { + ctx.ServerError("checking scope failed", err) + return + } + if !allow { + ctx.Error(http.StatusForbidden, "scopedAccessCheck", fmt.Sprintf("token does not have at least one of required scope(s): %v", requiredScope)) + return + } + } + repository, unitType, err := repo_service.LinkedRepository(ctx, attach) if err != nil { ctx.ServerError("LinkedRepository", err) @@ -110,7 +135,16 @@ func ServeAttachment(ctx *context.Context, uuid string) { return } } else { // If we have the repository we check access - perm, err := access_model.GetUserRepoPermission(ctx, repository, ctx.Doer) + // Some paths like `/attachment/{uuid}` can be accessed with API authentication mechanisms, and therefore *may* + // have AuthorizationReducers that need to be enforced: + reducer := ctx.Authentication.Reducer() + var perm access_model.Permission + var err error + if reducer != nil { + perm, err = access_model.GetUserRepoPermissionWithReducer(ctx, repository, ctx.Doer, reducer) + } else { + perm, err = access_model.GetUserRepoPermission(ctx, repository, ctx.Doer) + } if err != nil { ctx.Error(http.StatusInternalServerError, "GetUserRepoPermission", err.Error()) return diff --git a/routers/web/repo/githttp.go b/routers/web/repo/githttp.go index 936b108281..6703676745 100644 --- a/routers/web/repo/githttp.go +++ b/routers/web/repo/githttp.go @@ -184,8 +184,10 @@ func httpBase(ctx *context.Context) *serviceHandler { } if repoExist { - // Because of special ref "refs/for" .. , need delay write permission check - accessMode = perm.AccessModeRead + if !isWiki { + // Because of special ref "refs/for" .. , need delay write permission check + accessMode = perm.AccessModeRead + } if hasTaskID, taskID := ctx.Authentication.ActionsTaskID().Get(); hasTaskID { task, err := actions_model.GetTaskByID(ctx, taskID) diff --git a/services/context/permission.go b/services/context/permission.go index 44adbbd6d9..ad6843b440 100644 --- a/services/context/permission.go +++ b/services/context/permission.go @@ -4,6 +4,7 @@ package context import ( + "fmt" "net/http" "slices" @@ -57,6 +58,23 @@ func RequireRepoWriterOr(unitTypes ...unit.Type) func(ctx *Context) { // RequireRepoReader returns a middleware for requiring repository read to the specify unitType func RequireRepoReader(unitType unit.Type) func(ctx *Context) { return func(ctx *Context) { + // Typically checks for authentication scopes won't be relevant for non-API requests where this middleware is + // used; but, some paths like `/user/repo/raw/...` can be accessed with API authentication mechanisms. In those + // edge cases, check that `read:repository` scope is present if the authentication method indicates a limited + // scope. + hasScope, scope := ctx.Authentication.Scope().Get() + if hasScope { + allow, err := scope.HasScope(auth_model.AccessTokenScopeReadRepository) + if err != nil { + ctx.ServerError("checking scope failed", err) + return + } + if !allow { + ctx.Error(http.StatusForbidden, "scopedAccessCheck", fmt.Sprintf("token does not have at least one of required scope(s): %v", auth_model.AccessTokenScopeReadRepository)) + return + } + } + if !ctx.Repo.CanRead(unitType) { if log.IsTrace() { if ctx.IsSigned { diff --git a/services/context/repo.go b/services/context/repo.go index 583eb35474..f39b6d342d 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -375,7 +375,15 @@ func repoAssignment(ctx *Context, repo *repo_model.Repository) { return } - ctx.Repo.Permission, err = access_model.GetUserRepoPermission(ctx, repo, ctx.Doer) + // Typically checks for authorization reducers won't be relevant for non-API requests where this middleware is used; + // but, some paths like `/user/repo/raw/...` can be accessed with API authentication mechanisms. In those edge + // cases, initialize `ctx.Repo.Permission` based upon the reduced permission set available. + authorizationReducer := ctx.Authentication.Reducer() + if authorizationReducer == nil { + ctx.Repo.Permission, err = access_model.GetUserRepoPermission(ctx, repo, ctx.Doer) + } else { + ctx.Repo.Permission, err = access_model.GetUserRepoPermissionWithReducer(ctx, repo, ctx.Doer, authorizationReducer) + } if err != nil { ctx.ServerError("GetUserRepoPermission", err) return diff --git a/services/lfs/server.go b/services/lfs/server.go index 457602243e..1dfd58a3c0 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -607,6 +607,20 @@ func handleLFSToken(ctx stdCtx.Context, tokenSHA string, target *repo_model.Repo log.Error("Unable to GetUserById[%d]: Error: %v", claims.UserID, err) return nil, err } + + if !u.IsAccessAllowed(ctx) { + return nil, errors.New("user access is blocked") + } + + repoPerm, err := access_model.GetUserRepoPermission(ctx, target, u) + if err != nil { + log.Error("Unable to GetUserRepoPermission[%d]: Error: %v", claims.UserID, err) + return nil, err + } + if !repoPerm.CanAccess(mode, unit.TypeCode) { + return nil, errors.New("user does not have access to the repository") + } + return u, nil } diff --git a/services/lfs/server_test.go b/services/lfs/server_test.go index 065387ce08..875a99603e 100644 --- a/services/lfs/server_test.go +++ b/services/lfs/server_test.go @@ -78,6 +78,75 @@ func testAuthenticate(t *testing.T, cfg string) { assert.EqualValues(t, 2, u.ID) }) + t.Run("handleLFSToken nonexistent user", func(t *testing.T) { + tokenMissing, _ := getLFSAuthTokenWithBearer(authTokenOptions{Op: "download", UserID: 999, RepoID: 1}) + _, tokenMissing, _ = strings.Cut(tokenMissing, " ") + + u, err := handleLFSToken(ctx, tokenMissing, repo1, perm_model.AccessModeRead) + require.Error(t, err) + assert.Contains(t, err.Error(), "user does not exist") + assert.Nil(t, u) + }) + + t.Run("handleLFSToken nonexistent repo", func(t *testing.T) { + tokenBadRepo, _ := getLFSAuthTokenWithBearer(authTokenOptions{Op: "download", UserID: 2, RepoID: 999}) + _, tokenBadRepo, _ = strings.Cut(tokenBadRepo, " ") + badRepo := &repo_model.Repository{ID: 999} + + u, err := handleLFSToken(ctx, tokenBadRepo, badRepo, perm_model.AccessModeRead) + require.Error(t, err) + assert.Nil(t, u) + }) + + t.Run("handleLFSToken blocked user", func(t *testing.T) { + tokenBlocked, _ := getLFSAuthTokenWithBearer(authTokenOptions{Op: "download", UserID: 37, RepoID: 1}) + _, tokenBlocked, _ = strings.Cut(tokenBlocked, " ") + + u, err := handleLFSToken(ctx, tokenBlocked, repo1, perm_model.AccessModeRead) + require.Error(t, err) + assert.Contains(t, err.Error(), "user access is blocked") + assert.Nil(t, u) + }) + + t.Run("handleLFSToken no repo access", func(t *testing.T) { + repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) + tokenNoAccess, _ := getLFSAuthTokenWithBearer(authTokenOptions{Op: "download", UserID: 10, RepoID: 2}) + _, tokenNoAccess, _ = strings.Cut(tokenNoAccess, " ") + + u, err := handleLFSToken(ctx, tokenNoAccess, repo2, perm_model.AccessModeRead) + require.Error(t, err) + assert.Contains(t, err.Error(), "does not have access to the repository") + assert.Nil(t, u) + }) + + t.Run("handleLFSToken upload write access allowed", func(t *testing.T) { + tokenUploadRW, _ := getLFSAuthTokenWithBearer(authTokenOptions{Op: "upload", UserID: 2, RepoID: 1}) + _, tokenUploadRW, _ = strings.Cut(tokenUploadRW, " ") + + u, err := handleLFSToken(ctx, tokenUploadRW, repo1, perm_model.AccessModeWrite) + require.NoError(t, err) + assert.EqualValues(t, 2, u.ID) + }) + + t.Run("handleLFSToken upload read-only access denied", func(t *testing.T) { + tokenUploadRO, _ := getLFSAuthTokenWithBearer(authTokenOptions{Op: "upload", UserID: 10, RepoID: 1}) + _, tokenUploadRO, _ = strings.Cut(tokenUploadRO, " ") + + u, err := handleLFSToken(ctx, tokenUploadRO, repo1, perm_model.AccessModeWrite) + require.Error(t, err) + assert.Contains(t, err.Error(), "does not have access to the repository") + assert.Nil(t, u) + }) + + t.Run("handleLFSToken download read-only access allowed", func(t *testing.T) { + tokenDownloadRO, _ := getLFSAuthTokenWithBearer(authTokenOptions{Op: "download", UserID: 10, RepoID: 1}) + _, tokenDownloadRO, _ = strings.Cut(tokenDownloadRO, " ") + + u, err := handleLFSToken(ctx, tokenDownloadRO, repo1, perm_model.AccessModeRead) + require.NoError(t, err) + assert.EqualValues(t, 10, u.ID) + }) + t.Run("authenticate", func(t *testing.T) { const prefixBearer = "Bearer " assert.False(t, authenticate(ctx, repo1, "", true, false)) diff --git a/tests/integration/attachment_test.go b/tests/integration/attachment_test.go index 4e29407c2a..06c1bb82ca 100644 --- a/tests/integration/attachment_test.go +++ b/tests/integration/attachment_test.go @@ -13,9 +13,13 @@ import ( "strings" "testing" + auth_model "forgejo.org/models/auth" repo_model "forgejo.org/models/repo" + unit_model "forgejo.org/models/unit" + "forgejo.org/models/unittest" "forgejo.org/modules/storage" "forgejo.org/modules/test" + repo_service "forgejo.org/services/repository" "forgejo.org/tests" "github.com/stretchr/testify/assert" @@ -131,3 +135,226 @@ func TestGetAttachment(t *testing.T) { }) } } + +// Access under `/attachments/{uuid}` and `/{user}/{repo}/attachments/{uuid}` is permitted for API tokens. Those API +// tokens then need to have the read:issue or read:repository and the correct resource scopes to permit access, though. +func TestGetAttachmentViaAPITokens(t *testing.T) { + defer unittest.OverrideFixtures("tests/integration/fixtures/TestGetAttachmentViaAPITokens")() + defer tests.PrepareTestEnv(t)() + + // Create attachment data for an attachment added by this test's fixture. + _, err := storage.Attachments.Save(repo_model.AttachmentRelativePath("d962b49e-e32a-4b72-922d-33b551b629e2"), strings.NewReader("hello universe"), -1) + require.NoError(t, err) + + // Enable Issues unit on repo 16, one of our test targets. + repo_service.UpdateRepositoryUnits(t.Context(), + unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 16}), + []repo_model.RepoUnit{{ + RepoID: 16, + Type: unit_model.TypeIssues, + }}, nil) + + t.Run("attachments", func(t *testing.T) { + t.Run("no read:issue scope", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + session := loginUser(t, "user2") + allToken := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadMisc) + + t.Run("denied public repo1", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/attachments/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11").AddTokenAuth(allToken) + MakeRequest(t, req, http.StatusForbidden) + }) + t.Run("denied private repo2", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/attachments/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12").AddTokenAuth(allToken) + MakeRequest(t, req, http.StatusForbidden) + }) + // repo16 is a second repo used in fine-grain testing below, so we include it in other tests as a baseline + t.Run("denied private repo16", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/attachments/d962b49e-e32a-4b72-922d-33b551b629e2").AddTokenAuth(allToken) + MakeRequest(t, req, http.StatusForbidden) + }) + }) + + t.Run("all access token", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + session := loginUser(t, "user2") + allToken := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadIssue) + + t.Run("allowed public repo1", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/attachments/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11").AddTokenAuth(allToken) + resp := MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "hello world", resp.Body.String()) + }) + t.Run("allowed private repo2", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/attachments/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12").AddTokenAuth(allToken) + resp := MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "hello world", resp.Body.String()) + }) + // repo16 is a second repo used in fine-grain testing below, so we include it in other tests as a baseline + t.Run("allowed private repo16", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/attachments/d962b49e-e32a-4b72-922d-33b551b629e2").AddTokenAuth(allToken) + resp := MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "hello universe", resp.Body.String()) + }) + }) + + t.Run("public-only access token", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + session := loginUser(t, "user2") + publicOnlyToken := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopePublicOnly, auth_model.AccessTokenScopeReadIssue) + + t.Run("allowed public repo1", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/attachments/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11").AddTokenAuth(publicOnlyToken) + resp := MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "hello world", resp.Body.String()) + }) + t.Run("denied private repo2", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/attachments/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12").AddTokenAuth(publicOnlyToken) + MakeRequest(t, req, http.StatusNotFound) + }) + t.Run("denied private repo16", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/attachments/d962b49e-e32a-4b72-922d-33b551b629e2").AddTokenAuth(publicOnlyToken) + MakeRequest(t, req, http.StatusNotFound) + }) + }) + + t.Run("specific repo access token", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + repo2OnlyToken := createFineGrainedRepoAccessToken(t, "user2", + []auth_model.AccessTokenScope{auth_model.AccessTokenScopeReadIssue}, + []int64{2}, + ) + + t.Run("allowed public repo1", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/attachments/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11").AddTokenAuth(repo2OnlyToken) + resp := MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "hello world", resp.Body.String()) + }) + t.Run("allowed inside fine-grain repo2", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/attachments/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12").AddTokenAuth(repo2OnlyToken) + resp := MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "hello world", resp.Body.String()) + }) + t.Run("denied private outside fine-grain repo16", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/attachments/d962b49e-e32a-4b72-922d-33b551b629e2").AddTokenAuth(repo2OnlyToken) + MakeRequest(t, req, http.StatusNotFound) + }) + }) + }) + + t.Run("user-repo-attachments", func(t *testing.T) { + t.Run("no read:issue scope", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + session := loginUser(t, "user2") + allToken := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadMisc) + + t.Run("denied public repo1", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo1/attachments/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11").AddTokenAuth(allToken) + MakeRequest(t, req, http.StatusForbidden) + }) + t.Run("denied private repo2", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo2/attachments/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12").AddTokenAuth(allToken) + MakeRequest(t, req, http.StatusForbidden) + }) + // repo16 is a second repo used in fine-grain testing below, so we include it in other tests as a baseline + t.Run("denied private repo16", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo16/attachments/d962b49e-e32a-4b72-922d-33b551b629e2").AddTokenAuth(allToken) + MakeRequest(t, req, http.StatusForbidden) + }) + }) + + t.Run("all access token", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + session := loginUser(t, "user2") + allToken := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadIssue) + + t.Run("allowed public repo1", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo1/attachments/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11").AddTokenAuth(allToken) + resp := MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "hello world", resp.Body.String()) + }) + t.Run("allowed private repo2", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo2/attachments/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12").AddTokenAuth(allToken) + resp := MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "hello world", resp.Body.String()) + }) + // repo16 is a second repo used in fine-grain testing below, so we include it in other tests as a baseline + t.Run("allowed private repo16", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo16/attachments/d962b49e-e32a-4b72-922d-33b551b629e2").AddTokenAuth(allToken) + resp := MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "hello universe", resp.Body.String()) + }) + }) + + t.Run("public-only access token", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + session := loginUser(t, "user2") + publicOnlyToken := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopePublicOnly, auth_model.AccessTokenScopeReadIssue) + + t.Run("allowed public repo1", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo1/attachments/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11").AddTokenAuth(publicOnlyToken) + resp := MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "hello world", resp.Body.String()) + }) + t.Run("denied private repo2", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo2/attachments/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12").AddTokenAuth(publicOnlyToken) + MakeRequest(t, req, http.StatusNotFound) + }) + t.Run("denied private repo16", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo16/attachments/d962b49e-e32a-4b72-922d-33b551b629e2").AddTokenAuth(publicOnlyToken) + MakeRequest(t, req, http.StatusNotFound) + }) + }) + + t.Run("specific repo access token", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + repo2OnlyToken := createFineGrainedRepoAccessToken(t, "user2", + []auth_model.AccessTokenScope{auth_model.AccessTokenScopeReadIssue}, + []int64{2}, + ) + + t.Run("allowed public repo1", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo1/attachments/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11").AddTokenAuth(repo2OnlyToken) + resp := MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "hello world", resp.Body.String()) + }) + t.Run("allowed inside fine-grain repo2", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo2/attachments/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12").AddTokenAuth(repo2OnlyToken) + resp := MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "hello world", resp.Body.String()) + }) + t.Run("denied private outside fine-grain repo16", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo16/attachments/d962b49e-e32a-4b72-922d-33b551b629e2").AddTokenAuth(repo2OnlyToken) + MakeRequest(t, req, http.StatusNotFound) + }) + }) + }) +} diff --git a/tests/integration/download_test.go b/tests/integration/download_test.go index d5e68402c5..70acc7f16f 100644 --- a/tests/integration/download_test.go +++ b/tests/integration/download_test.go @@ -7,6 +7,7 @@ import ( "net/http" "testing" + auth_model "forgejo.org/models/auth" "forgejo.org/modules/setting" "forgejo.org/tests" @@ -15,7 +16,6 @@ import ( func TestDownloadByID(t *testing.T) { defer tests.PrepareTestEnv(t)() - session := loginUser(t, "user2") // Request raw blob @@ -91,3 +91,110 @@ func TestDownloadRawTextFileWithMimeTypeMapping(t *testing.T) { delete(setting.MimeTypeMap.Map, ".xml") setting.MimeTypeMap.Enabled = false } + +// Access under `/raw` is permitted for API tokens. Those API tokens then need to have the read:repository and the +// correct resource scopes to permit access, though. The below series of tests covers the middleware combinations on +// the entire `/user/repo/raw/*` URL tree as they use a common middleware implementation. +func TestDownloadAccessViaAPITokens(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + t.Run("no read:repository scope", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + session := loginUser(t, "user2") + allToken := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadMisc) + + t.Run("denied public repo1", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo1/raw/blob/4b4851ad51df6a7d9f25c979345979eaeb5b349f").AddTokenAuth(allToken) + MakeRequest(t, req, http.StatusForbidden) + }) + t.Run("denied private repo2", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo2/raw/blob/1032bbf17fbc0d9c95bb5418dabe8f8c99278700").AddTokenAuth(allToken) + MakeRequest(t, req, http.StatusForbidden) + }) + // repo16 is a second repo used in fine-grain testing below, so we include it in other tests as a baseline + t.Run("denied private repo16", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo16/raw/blob/69554a64c1e6030f051e5c3f94bfbd773cd6a324").AddTokenAuth(allToken) + MakeRequest(t, req, http.StatusForbidden) + }) + }) + + t.Run("all access token", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + session := loginUser(t, "user2") + allToken := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadRepository) + + t.Run("allowed public repo1", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo1/raw/blob/4b4851ad51df6a7d9f25c979345979eaeb5b349f").AddTokenAuth(allToken) + resp := MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "# repo1\n\nDescription for repo1", resp.Body.String()) + }) + t.Run("allowed private repo2", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo2/raw/blob/1032bbf17fbc0d9c95bb5418dabe8f8c99278700").AddTokenAuth(allToken) + resp := MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "tree ba1aed4e2ea2443d76cec241b96be4ec990852ec\nparent 205ac761f3326a7ebe416e8673760016450b5cec\nauthor Jimmy Praet 1624996449 +0200\ncommitter Jimmy Praet 1624996449 +0200\n\nAdd test.xml\n", resp.Body.String()) + }) + // repo16 is a second repo used in fine-grain testing below, so we include it in other tests as a baseline + t.Run("allowed private repo16", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo16/raw/blob/69554a64c1e6030f051e5c3f94bfbd773cd6a324").AddTokenAuth(allToken) + resp := MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "tree 24f83a471f77579fea57bac7255d6e64e70fce1c\nparent 27566bd5738fc8b4e3fef3c5e72cce608537bd95\nauthor User2 1502042309 +0200\ncommitter User2 1502042309 +0200\n\nnot signed commit\n", resp.Body.String()) + }) + }) + + t.Run("public-only access token", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + session := loginUser(t, "user2") + publicOnlyToken := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopePublicOnly, auth_model.AccessTokenScopeReadRepository) + + t.Run("allowed public repo1", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo1/raw/blob/4b4851ad51df6a7d9f25c979345979eaeb5b349f").AddTokenAuth(publicOnlyToken) + resp := MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "# repo1\n\nDescription for repo1", resp.Body.String()) + }) + t.Run("denied private repo2", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo2/raw/blob/1032bbf17fbc0d9c95bb5418dabe8f8c99278700").AddTokenAuth(publicOnlyToken) + MakeRequest(t, req, http.StatusNotFound) + }) + t.Run("denied private repo16", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo16/raw/blob/69554a64c1e6030f051e5c3f94bfbd773cd6a324").AddTokenAuth(publicOnlyToken) + MakeRequest(t, req, http.StatusNotFound) + }) + }) + + t.Run("specific repo access token", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + repo2OnlyToken := createFineGrainedRepoAccessToken(t, "user2", + []auth_model.AccessTokenScope{auth_model.AccessTokenScopeReadRepository}, + []int64{2}, + ) + + t.Run("allowed public repo1", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo1/raw/blob/4b4851ad51df6a7d9f25c979345979eaeb5b349f").AddTokenAuth(repo2OnlyToken) + resp := MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "# repo1\n\nDescription for repo1", resp.Body.String()) + }) + t.Run("allowed inside fine-grain repo2", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo2/raw/blob/1032bbf17fbc0d9c95bb5418dabe8f8c99278700").AddTokenAuth(repo2OnlyToken) + resp := MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "tree ba1aed4e2ea2443d76cec241b96be4ec990852ec\nparent 205ac761f3326a7ebe416e8673760016450b5cec\nauthor Jimmy Praet 1624996449 +0200\ncommitter Jimmy Praet 1624996449 +0200\n\nAdd test.xml\n", resp.Body.String()) + }) + t.Run("denied private outside fine-grain repo16", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo16/raw/blob/69554a64c1e6030f051e5c3f94bfbd773cd6a324").AddTokenAuth(repo2OnlyToken) + MakeRequest(t, req, http.StatusNotFound) + }) + }) +} diff --git a/tests/integration/fixtures/TestGetAttachmentViaAPITokens/attachment.yml b/tests/integration/fixtures/TestGetAttachmentViaAPITokens/attachment.yml new file mode 100644 index 0000000000..334b33cefa --- /dev/null +++ b/tests/integration/fixtures/TestGetAttachmentViaAPITokens/attachment.yml @@ -0,0 +1,12 @@ +- + id: 50 + uuid: d962b49e-e32a-4b72-922d-33b551b629e2 + repo_id: 16 + issue_id: 50 + release_id: 0 + uploader_id: 0 + comment_id: 0 + name: attach1 + download_count: 0 + size: 0 + created_unix: 946684800 diff --git a/tests/integration/fixtures/TestGetAttachmentViaAPITokens/issue.yml b/tests/integration/fixtures/TestGetAttachmentViaAPITokens/issue.yml new file mode 100644 index 0000000000..7a876ef5b2 --- /dev/null +++ b/tests/integration/fixtures/TestGetAttachmentViaAPITokens/issue.yml @@ -0,0 +1,16 @@ +- + id: 50 + repo_id: 16 + index: 1 + poster_id: 1 + original_author_id: 0 + name: issue1 + content: content for the first issue + milestone_id: 0 + priority: 0 + is_closed: false + is_pull: false + num_comments: 3 + created_unix: 946684800 + updated_unix: 978307200 + is_locked: false diff --git a/tests/integration/oauth_test.go b/tests/integration/oauth_test.go index 3f14324cf7..d87d725dbd 100644 --- a/tests/integration/oauth_test.go +++ b/tests/integration/oauth_test.go @@ -194,12 +194,45 @@ func TestAccessTokenExchange(t *testing.T) { assert.Greater(t, len(parsed.RefreshToken), 10) } +func TestAccessTokenExchangeRedirectURIMismatch(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + // The auth code fixture has redirect_uri="a", but we send a different + // URI that is registered with the app ("https://example.com/xyzzy"). + // Per RFC 6749 §4.1.3, this must be rejected. + req := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "authorization_code", + "client_id": "da7da3ba-9a13-4167-856f-3899de0b0138", + "client_secret": "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=", + "redirect_uri": "https://example.com/xyzzy", + "code": "authcode", + "code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt", + }) + resp := MakeRequest(t, req, http.StatusBadRequest) + + var parsedError auth.AccessTokenErrorResponse + require.NoError(t, json.Unmarshal(resp.Body.Bytes(), &parsedError)) + assert.Equal(t, "unauthorized_client", string(parsedError.ErrorCode)) + assert.Equal(t, "redirect_uri does not match the authorization request", parsedError.ErrorDescription) + + // Using the correct redirect_uri ("a") should succeed + req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "authorization_code", + "client_id": "da7da3ba-9a13-4167-856f-3899de0b0138", + "client_secret": "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=", + "redirect_uri": "a", + "code": "authcode", + "code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt", + }) + MakeRequest(t, req, http.StatusOK) +} + func TestAccessTokenExchangeWithPublicClient(t *testing.T) { defer tests.PrepareTestEnv(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", }) @@ -497,6 +530,56 @@ func TestRefreshTokenInvalidation(t *testing.T) { assert.Equal(t, "token was already used", parsedError.ErrorDescription) } +func TestRefreshTokenCrossClientUsage(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + // Step 1: Obtain a refresh token via app 1 (confidential client) + req := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "authorization_code", + "client_id": "da7da3ba-9a13-4167-856f-3899de0b0138", + "client_secret": "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=", + "redirect_uri": "a", + "code": "authcode", + "code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt", + }) + resp := MakeRequest(t, req, http.StatusOK) + + type tokenResponse struct { + AccessToken string `json:"access_token"` + TokenType string `json:"token_type"` + ExpiresIn int64 `json:"expires_in"` + RefreshToken string `json:"refresh_token"` + } + parsed := new(tokenResponse) + require.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsed)) + assert.NotEmpty(t, parsed.RefreshToken) + + // Step 2: Try to use the refresh token with app 2 (different client): must fail + req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "refresh_token", + "client_id": "ce5a1322-42a7-11ed-b878-0242ac120002", + "client_secret": "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=", + "redirect_uri": "b", + "refresh_token": parsed.RefreshToken, + }) + resp = MakeRequest(t, req, http.StatusBadRequest) + + var parsedError auth.AccessTokenErrorResponse + require.NoError(t, json.Unmarshal(resp.Body.Bytes(), &parsedError)) + assert.Equal(t, "invalid_grant", string(parsedError.ErrorCode)) + assert.Equal(t, "refresh token was not issued to this client", parsedError.ErrorDescription) + + // Step 3: Using the refresh token with the correct app 1 should still work + req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "refresh_token", + "client_id": "da7da3ba-9a13-4167-856f-3899de0b0138", + "client_secret": "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=", + "redirect_uri": "a", + "refresh_token": parsed.RefreshToken, + }) + MakeRequest(t, req, http.StatusOK) +} + func TestSignInOAuthCallbackSignIn(t *testing.T) { defer tests.PrepareTestEnv(t)() diff --git a/tests/integration/wiki_test.go b/tests/integration/wiki_test.go index 47986177d7..1fd9378852 100644 --- a/tests/integration/wiki_test.go +++ b/tests/integration/wiki_test.go @@ -4,19 +4,29 @@ package integration import ( + "encoding/base64" "fmt" + "io" "net/http" "net/url" "os" "path/filepath" "strings" "testing" + "time" + auth_model "forgejo.org/models/auth" + unit_model "forgejo.org/models/unit" + "forgejo.org/models/unittest" + user_model "forgejo.org/models/user" "forgejo.org/modules/git" + "forgejo.org/modules/optional" + api "forgejo.org/modules/structs" "forgejo.org/modules/util" "forgejo.org/tests" "github.com/PuerkitoBio/goquery" + "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -24,7 +34,16 @@ import ( func assertFileExist(t *testing.T, p string) { exist, err := util.IsExist(p) require.NoError(t, err) - assert.True(t, exist) + if !assert.True(t, exist) { + dir := filepath.Dir(p) + t.Logf("Listing files that were present in dir path %s", dir) + entries, err := os.ReadDir(dir) + require.NoError(t, err) + for _, e := range entries { + t.Logf("file in path %s -> %s", dir, e.Name()) + } + t.Logf("End of %d entries in directory %s", len(entries), dir) + } } func assertFileEqual(t *testing.T, p string, content []byte) { @@ -33,24 +52,162 @@ func assertFileEqual(t *testing.T, p string, content []byte) { assert.Equal(t, content, bs) } -func TestRepoCloneWiki(t *testing.T) { - onApplicationRun(t, func(t *testing.T, u *url.URL) { - dstPath := t.TempDir() +type ( + RepoWikiMethod string + RepoWikiAuth string + RepoWikiTarget string + RepoWikiOperation string +) - r := fmt.Sprintf("%suser2/repo1.wiki.git", u.String()) - u, _ = url.Parse(r) - u.User = url.UserPassword("user2", userPassword) - t.Run("Clone", func(t *testing.T) { - require.NoError(t, git.CloneWithArgs(t.Context(), git.AllowLFSFiltersArgs(), u.String(), dstPath, git.CloneRepoOptions{})) - assertFileEqual(t, filepath.Join(dstPath, "Home.md"), []byte("# Home page\n\nThis is the home page!\n")) - assertFileExist(t, filepath.Join(dstPath, "Page-With-Image.md")) - assertFileExist(t, filepath.Join(dstPath, "Page-With-Spaced-Name.md")) - assertFileExist(t, filepath.Join(dstPath, "images")) - assertFileExist(t, filepath.Join(dstPath, "jpeg.jpg")) - }) +const ( + RepoWikiSSH RepoWikiMethod = "SSH" + RepoWikiHTTP RepoWikiMethod = "HTTP" + + RepoWikiAnonymous RepoWikiAuth = "Anonymous" + RepoWikiAuthenticated RepoWikiAuth = "Authenticated" + RepoWikiAuthenticatedNonOwnerUser RepoWikiAuth = "Authenticated-NonOwner" + + RepoWikiPublic RepoWikiTarget = "Public" + RepoWikiPrivate RepoWikiTarget = "Private" + + RepoWikiRead RepoWikiOperation = "Read" + RepoWikiWrite RepoWikiOperation = "Write" +) + +func TestRepoWikiGitOperation(t *testing.T) { + onApplicationRun(t, func(t *testing.T, u *url.URL) { + for _, method := range []RepoWikiMethod{RepoWikiSSH, RepoWikiHTTP} { + for _, auth := range []RepoWikiAuth{RepoWikiAnonymous, RepoWikiAuthenticated, RepoWikiAuthenticatedNonOwnerUser} { + for _, target := range []RepoWikiTarget{RepoWikiPublic, RepoWikiPrivate} { + for _, operation := range []RepoWikiOperation{RepoWikiRead, RepoWikiWrite} { + t.Run(fmt.Sprintf("%s/%s/%s/%s", method, auth, target, operation), func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + doRepoWikiGitOperation(t, u, method, auth, target, operation) + }) + } + } + } + } }) } +func doRepoWikiGitOperation(t *testing.T, serverURL *url.URL, method RepoWikiMethod, auth RepoWikiAuth, target RepoWikiTarget, operation RepoWikiOperation) { + repo := "repo1" + if target == RepoWikiPrivate { + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + privateRepo, _, reset := tests.CreateDeclarativeRepoWithOptions(t, user2, tests.DeclarativeRepoOptions{ + IsPrivate: optional.Some(true), + EnabledUnits: optional.Some([]unit_model.Type{unit_model.TypeWiki}), + }) + defer reset() + + session := loginUser(t, user2.LoginName) + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) + urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/wiki/new", user2.LoginName, privateRepo.Name) + req := NewRequestWithJSON(t, "POST", urlStr, &api.CreateWikiPageOptions{ + Title: "Page With Image", + ContentBase64: base64.StdEncoding.EncodeToString([]byte("# Page With Image\n\n![Gitea Logo](./raw/jpeg.jpg)\n")), + Message: "", + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusCreated) + + repo = privateRepo.Name + } + + dstPath := t.TempDir() + r := fmt.Sprintf("%suser2/%s.wiki.git", serverURL.String(), repo) + testURL, err := url.Parse(r) + require.NoError(t, err) + + if method == RepoWikiHTTP { + switch auth { + case RepoWikiAnonymous: + // no-op + case RepoWikiAuthenticated: + testURL.User = url.UserPassword("user2", userPassword) + case RepoWikiAuthenticatedNonOwnerUser: + testURL.User = url.UserPassword("user20", userPassword) + default: + t.Fatalf("unexpected auth = %s", auth) + } + + doRepoWikiGitOperationInner(t, testURL, dstPath, auth, target, operation) + } else if method == RepoWikiSSH { + var user string + switch auth { + case RepoWikiAnonymous: + t.Skip() // anonymous ssh is not supported + case RepoWikiAuthenticated: + user = "user2" // owner of the repo + case RepoWikiAuthenticatedNonOwnerUser: + user = "user20" // not the owner of the repo, not a collaborator + default: + t.Fatalf("unexpected auth = %s", auth) + } + + keyname := "my-testing-key" + withKeyFile(t, keyname, func(keyFile string) { + baseAPITestContext := NewAPITestContext(t, user, repo, auth_model.AccessTokenScopeWriteUser) + t.Run("CreateUserKey", doAPICreateUserKey(baseAPITestContext, fmt.Sprintf("test-key-%s", uuid.New().String()), keyFile, func(t *testing.T, pk api.PublicKey) {})) + + baseAPITestContext.Username = "user2" // target repo owner to compose URLs + baseAPITestContext.Reponame = fmt.Sprintf("%s.wiki", repo) + testURL = createSSHUrl(baseAPITestContext.GitPath(), testURL) + + doRepoWikiGitOperationInner(t, testURL, dstPath, auth, target, operation) + }) + } else { + t.Fatalf("unexpected method = %s", method) + } +} + +func doRepoWikiGitOperationInner(t *testing.T, gitURL *url.URL, dstPath string, auth RepoWikiAuth, target RepoWikiTarget, operation RepoWikiOperation) { + err := git.CloneWithArgs(t.Context(), git.AllowLFSFiltersArgs(), gitURL.String(), dstPath, git.CloneRepoOptions{}) + if target == RepoWikiPrivate && (auth == RepoWikiAnonymous || auth == RepoWikiAuthenticatedNonOwnerUser) { + require.Error(t, err, "clone must fail; auth %s shouldn't be able to access private repo") + return // no other test conditions to satisfy if the clone failed + } + require.NoError(t, err, "clone must succeed; auth %s should be able to access a public repo") + + assertFileExist(t, filepath.Join(dstPath, "Page-With-Image.md")) + assertFileEqual(t, filepath.Join(dstPath, "Page-With-Image.md"), []byte("# Page With Image\n\n![Gitea Logo](./raw/jpeg.jpg)\n")) + + if operation == RepoWikiWrite { + f, err := os.OpenFile(filepath.Join(dstPath, "Home.md"), os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0o644) + defer f.Close() + require.NoError(t, err) + _, err = io.WriteString(f, fmt.Sprintf("# Home Page Edited!\n%s", uuid.New().String())) + require.NoError(t, err) + err = f.Close() + require.NoError(t, err) + + err = git.AddChanges(dstPath, true) + require.NoError(t, err) + err = git.CommitChanges(dstPath, git.CommitChangesOptions{Message: "Changes made!"}) + require.NoError(t, err) + + // don't use git.Push() because it doesn't support credential helper, and 'origin' would have had its URL saved + // with the creds stripped in dstPath so we need the credential helper to be configured. + cmd := git.NewCommand(t.Context()) + if gitURL.Scheme == "http" { + _, credCleanup, err := cmd.AddAuthCredentialHelperForRemote(gitURL.String()) + require.NoError(t, err) + defer credCleanup() + } + cmd.AddArguments("push", "origin") + + stdout, stderr, err := cmd.RunStdString(&git.RunOpts{ + Dir: dstPath, + Timeout: 2 * time.Second, + }) + if auth == RepoWikiAuthenticated { + require.NoError(t, err, "stdout = %q, stderr = %q", stdout, stderr) + } else { + require.Error(t, err, "push must fail as authentication mode %s doesn't allow write, but succeeded. stdout = %q, stderr = %q", auth, stdout, stderr) + } + } +} + func Test_RepoWikiPages(t *testing.T) { defer tests.PrepareTestEnv(t)()