Replace http types on forge interface (#3374)

server/api/login.go

@@ -25,6 +25,7 @@ import (
 	"github.com/rs/zerolog/log"
 
 	"go.woodpecker-ci.org/woodpecker/v2/server"
+	forge_types "go.woodpecker-ci.org/woodpecker/v2/server/forge/types"
 	"go.woodpecker-ci.org/woodpecker/v2/server/model"
 	"go.woodpecker-ci.org/woodpecker/v2/server/store"
 	"go.woodpecker-ci.org/woodpecker/v2/server/store/types"
@@ -48,15 +49,20 @@ func HandleAuth(c *gin.Context) {
 	// cannot, however, remember why, so need to revisit this line.
 	c.Writer.Header().Del("Content-Type")
 
-	tmpuser, err := _forge.Login(c, c.Writer, c.Request)
+	tmpuser, redirectURL, err := _forge.Login(c, &forge_types.OAuthRequest{
+		Error:            c.Request.FormValue("error"),
+		ErrorURI:         c.Request.FormValue("error_uri"),
+		ErrorDescription: c.Request.FormValue("error_description"),
+		Code:             c.Request.FormValue("code"),
+	})
 	if err != nil {
 		log.Error().Err(err).Msg("cannot authenticate user")
 		c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=oauth_error")
 		return
 	}
-	// this will happen when the forge redirects the user as
+	// The user is not authorized yet -> redirect
-	// part of the authorization workflow.
 	if tmpuser == nil {
+		http.Redirect(c.Writer, c.Request, redirectURL, http.StatusSeeOther)
 		return
 	}
 

server/forge/bitbucket/bitbucket.go

@@ -77,36 +77,35 @@ func (c *config) URL() string {
 
 // Login authenticates an account with Bitbucket using the oauth2 protocol. The
 // Bitbucket account details are returned when the user is successfully authenticated.
-func (c *config) Login(ctx context.Context, w http.ResponseWriter, req *http.Request) (*model.User, error) {
+func (c *config) Login(ctx context.Context, req *forge_types.OAuthRequest) (*model.User, string, error) {
 	config := c.newOAuth2Config()
+	redirectURL := config.AuthCodeURL("woodpecker")
 
 	// get the OAuth errors
-	if err := req.FormValue("error"); err != "" {
+	if req.Error != "" {
-		return nil, &forge_types.AuthError{
+		return nil, redirectURL, &forge_types.AuthError{
-			Err:         err,
+			Err:         req.Error,
-			Description: req.FormValue("error_description"),
+			Description: req.ErrorDescription,
-			URI:         req.FormValue("error_uri"),
+			URI:         req.ErrorURI,
 		}
 	}
 
-	// get the OAuth code
+	// check the OAuth code
-	code := req.FormValue("code")
+	if len(req.Code) == 0 {
-	if len(code) == 0 {
+		return nil, redirectURL, nil
-		http.Redirect(w, req, config.AuthCodeURL("woodpecker"), http.StatusSeeOther)
-		return nil, nil
 	}
 
-	token, err := config.Exchange(ctx, code)
+	token, err := config.Exchange(ctx, req.Code)
 	if err != nil {
-		return nil, err
+		return nil, redirectURL, err
 	}
 
 	client := internal.NewClient(ctx, c.API, config.Client(ctx, token))
 	curr, err := client.FindCurrent()
 	if err != nil {
-		return nil, err
+		return nil, redirectURL, err
 	}
-	return convertUser(curr, token), nil
+	return convertUser(curr, token), redirectURL, nil
 }
 
 // Auth uses the Bitbucket oauth2 access token and refresh token to authenticate

server/forge/bitbucket/bitbucket_test.go

@@ -27,6 +27,7 @@ import (
 
 	"go.woodpecker-ci.org/woodpecker/v2/server/forge/bitbucket/fixtures"
 	"go.woodpecker-ci.org/woodpecker/v2/server/forge/bitbucket/internal"
+	"go.woodpecker-ci.org/woodpecker/v2/server/forge/types"
 	"go.woodpecker-ci.org/woodpecker/v2/server/model"
 )
 
@@ -63,34 +64,35 @@ func Test_bitbucket(t *testing.T) {
 
 		g.Describe("Given an authorization request", func() {
 			g.It("Should redirect to authorize", func() {
-				w := httptest.NewRecorder()
+				user, _, err := c.Login(ctx, &types.OAuthRequest{})
-				r, _ := http.NewRequest("GET", "", nil)
-				_, err := c.Login(ctx, w, r)
 				g.Assert(err).IsNil()
-				g.Assert(w.Code).Equal(http.StatusSeeOther)
+				g.Assert(user).IsNil()
 			})
 			g.It("Should return authenticated user", func() {
-				r, _ := http.NewRequest("GET", "?code=code", nil)
+				u, _, err := c.Login(ctx, &types.OAuthRequest{
-				u, err := c.Login(ctx, nil, r)
+					Code: "code",
+				})
 				g.Assert(err).IsNil()
 				g.Assert(u.Login).Equal(fakeUser.Login)
 				g.Assert(u.Token).Equal("2YotnFZFEjr1zCsicMWpAA")
 				g.Assert(u.Secret).Equal("tGzv3JOkF0XG5Qx2TlKWIA")
 			})
 			g.It("Should handle failure to exchange code", func() {
-				w := httptest.NewRecorder()
+				_, _, err := c.Login(ctx, &types.OAuthRequest{
-				r, _ := http.NewRequest("GET", "?code=code_bad_request", nil)
+					Code: "code_bad_request",
-				_, err := c.Login(ctx, w, r)
+				})
 				g.Assert(err).IsNotNil()
 			})
 			g.It("Should handle failure to resolve user", func() {
-				r, _ := http.NewRequest("GET", "?code=code_user_not_found", nil)
+				_, _, err := c.Login(ctx, &types.OAuthRequest{
-				_, err := c.Login(ctx, nil, r)
+					Code: "code_user_not_found",
+				})
 				g.Assert(err).IsNotNil()
 			})
 			g.It("Should handle authentication errors", func() {
-				r, _ := http.NewRequest("GET", "?error=invalid_scope", nil)
+				_, _, err := c.Login(ctx, &types.OAuthRequest{
-				_, err := c.Login(ctx, nil, r)
+					Error: "invalid_scope",
+				})
 				g.Assert(err).IsNotNil()
 			})
 		})

server/forge/forge.go

@@ -36,8 +36,8 @@ type Forge interface {
 	URL() string
 
 	// Login authenticates the session and returns the
-	// forge user details.
+	// forge user details and the URL to redirect to if not authorized yet.
-	Login(ctx context.Context, w http.ResponseWriter, r *http.Request) (*model.User, error)
+	Login(ctx context.Context, r *types.OAuthRequest) (*model.User, string, error)
 
 	// Auth authenticates the session and returns the forge user
 	// login for the given token and secret

server/forge/gitea/gitea.go

@@ -116,37 +116,36 @@ func (c *Gitea) oauth2Config(ctx context.Context) (*oauth2.Config, context.Conte
 
 // Login authenticates an account with Gitea using basic authentication. The
 // Gitea account details are returned when the user is successfully authenticated.
-func (c *Gitea) Login(ctx context.Context, w http.ResponseWriter, req *http.Request) (*model.User, error) {
+func (c *Gitea) Login(ctx context.Context, req *forge_types.OAuthRequest) (*model.User, string, error) {
 	config, oauth2Ctx := c.oauth2Config(ctx)
+	redirectURL := config.AuthCodeURL("woodpecker")
 
-	// get the OAuth errors
+	// check the OAuth errors
-	if err := req.FormValue("error"); err != "" {
+	if req.Error != "" {
-		return nil, &forge_types.AuthError{
+		return nil, redirectURL, &forge_types.AuthError{
-			Err:         err,
+			Err:         req.Error,
-			Description: req.FormValue("error_description"),
+			Description: req.ErrorDescription,
-			URI:         req.FormValue("error_uri"),
+			URI:         req.ErrorURI,
 		}
 	}
 
-	// get the OAuth code
+	// check the OAuth code
-	code := req.FormValue("code")
+	if len(req.Code) == 0 {
-	if len(code) == 0 {
+		return nil, redirectURL, nil
-		http.Redirect(w, req, config.AuthCodeURL("woodpecker"), http.StatusSeeOther)
-		return nil, nil
 	}
 
-	token, err := config.Exchange(oauth2Ctx, code)
+	token, err := config.Exchange(oauth2Ctx, req.Code)
 	if err != nil {
-		return nil, err
+		return nil, redirectURL, err
 	}
 
 	client, err := c.newClientToken(ctx, token.AccessToken)
 	if err != nil {
-		return nil, err
+		return nil, redirectURL, err
 	}
 	account, _, err := client.GetMyUserInfo()
 	if err != nil {
-		return nil, err
+		return nil, redirectURL, err
 	}
 
 	return &model.User{
@@ -157,7 +156,7 @@ func (c *Gitea) Login(ctx context.Context, w http.ResponseWriter, req *http.Requ
 		Email:         account.Email,
 		ForgeRemoteID: model.ForgeRemoteID(fmt.Sprint(account.ID)),
 		Avatar:        expandAvatar(c.url, account.AvatarURL),
-	}, nil
+	}, redirectURL, nil
 }
 
 // Auth uses the Gitea oauth2 access token and refresh token to authenticate

server/forge/github/github.go

@@ -92,46 +92,45 @@ func (c *client) URL() string {
 }
 
 // Login authenticates the session and returns the forge user details.
-func (c *client) Login(ctx context.Context, res http.ResponseWriter, req *http.Request) (*model.User, error) {
+func (c *client) Login(ctx context.Context, req *forge_types.OAuthRequest) (*model.User, string, error) {
-	config := c.newConfig(req)
+	config := c.newConfig()
+	redirectURL := config.AuthCodeURL("woodpecker")
 
-	// get the OAuth errors
+	// check the OAuth errors
-	if err := req.FormValue("error"); err != "" {
+	if req.Error != "" {
-		return nil, &forge_types.AuthError{
+		return nil, redirectURL, &forge_types.AuthError{
-			Err:         err,
+			Err:         req.Error,
-			Description: req.FormValue("error_description"),
+			Description: req.ErrorDescription,
-			URI:         req.FormValue("error_uri"),
+			URI:         req.ErrorURI,
 		}
 	}
 
-	// get the OAuth code
+	// check the OAuth code
-	code := req.FormValue("code")
+	if len(req.Code) == 0 {
-	if len(code) == 0 {
 		// TODO(bradrydzewski) we really should be using a random value here and
 		// storing in a cookie for verification in the next stage of the workflow.
 
-		http.Redirect(res, req, config.AuthCodeURL("woodpecker"), http.StatusSeeOther)
+		return nil, redirectURL, nil
-		return nil, nil
 	}
 
-	token, err := config.Exchange(c.newContext(ctx), code)
+	token, err := config.Exchange(c.newContext(ctx), req.Code)
 	if err != nil {
-		return nil, err
+		return nil, redirectURL, err
 	}
 
 	client := c.newClientToken(ctx, token.AccessToken)
 	user, _, err := client.Users.Get(ctx, "")
 	if err != nil {
-		return nil, err
+		return nil, redirectURL, err
 	}
 
 	emails, _, err := client.Users.ListEmails(ctx, nil)
 	if err != nil {
-		return nil, err
+		return nil, redirectURL, err
 	}
 	email := matchingEmail(emails, c.API)
 	if email == nil {
-		return nil, fmt.Errorf("no verified Email address for GitHub account")
+		return nil, redirectURL, fmt.Errorf("no verified Email address for GitHub account")
 	}
 
 	return &model.User{
@@ -140,7 +139,7 @@ func (c *client) Login(ctx context.Context, res http.ResponseWriter, req *http.R
 		Token:         token.AccessToken,
 		Avatar:        user.GetAvatarURL(),
 		ForgeRemoteID: model.ForgeRemoteID(fmt.Sprint(user.GetID())),
-	}, nil
+	}, redirectURL, nil
 }
 
 // Auth returns the GitHub user login for the given access token.
@@ -405,16 +404,7 @@ func (c *client) newContext(ctx context.Context) context.Context {
 }
 
 // helper function to return the GitHub oauth2 config
-func (c *client) newConfig(req *http.Request) *oauth2.Config {
+func (c *client) newConfig() *oauth2.Config {
-	var redirect string
-
-	intendedURL := req.URL.Query()["url"]
-	if len(intendedURL) > 0 {
-		redirect = fmt.Sprintf("%s/authorize?url=%s", server.Config.Server.OAuthHost, intendedURL[0])
-	} else {
-		redirect = fmt.Sprintf("%s/authorize", server.Config.Server.OAuthHost)
-	}
-
 	return &oauth2.Config{
 		ClientID:     c.Client,
 		ClientSecret: c.Secret,
@@ -423,7 +413,7 @@ func (c *client) newConfig(req *http.Request) *oauth2.Config {
 			AuthURL:  fmt.Sprintf("%s/login/oauth/authorize", c.url),
 			TokenURL: fmt.Sprintf("%s/login/oauth/access_token", c.url),
 		},
-		RedirectURL: redirect,
+		RedirectURL: fmt.Sprintf("%s/authorize", server.Config.Server.OAuthHost),
 	}
 }
 

server/forge/gitlab/gitlab.go

@@ -105,38 +105,37 @@ func (g *GitLab) oauth2Config(ctx context.Context) (*oauth2.Config, context.Cont
 
 // Login authenticates the session and returns the
 // forge user details.
-func (g *GitLab) Login(ctx context.Context, res http.ResponseWriter, req *http.Request) (*model.User, error) {
+func (g *GitLab) Login(ctx context.Context, req *forge_types.OAuthRequest) (*model.User, string, error) {
 	config, oauth2Ctx := g.oauth2Config(ctx)
+	redirectURL := config.AuthCodeURL("woodpecker")
 
-	// get the OAuth errors
+	// check the OAuth errors
-	if err := req.FormValue("error"); err != "" {
+	if req.Error != "" {
-		return nil, &forge_types.AuthError{
+		return nil, redirectURL, &forge_types.AuthError{
-			Err:         err,
+			Err:         req.Error,
-			Description: req.FormValue("error_description"),
+			Description: req.ErrorDescription,
-			URI:         req.FormValue("error_uri"),
+			URI:         req.ErrorURI,
 		}
 	}
 
-	// get the OAuth code
+	// check the OAuth code
-	code := req.FormValue("code")
+	if len(req.Code) == 0 {
-	if len(code) == 0 {
+		return nil, redirectURL, nil
-		http.Redirect(res, req, config.AuthCodeURL("woodpecker"), http.StatusSeeOther)
-		return nil, nil
 	}
 
-	token, err := config.Exchange(oauth2Ctx, code)
+	token, err := config.Exchange(oauth2Ctx, req.Code)
 	if err != nil {
-		return nil, fmt.Errorf("error exchanging token: %w", err)
+		return nil, redirectURL, fmt.Errorf("error exchanging token: %w", err)
 	}
 
 	client, err := newClient(g.url, token.AccessToken, g.SkipVerify)
 	if err != nil {
-		return nil, err
+		return nil, redirectURL, err
 	}
 
 	login, _, err := client.Users.CurrentUser(gitlab.WithContext(ctx))
 	if err != nil {
-		return nil, err
+		return nil, redirectURL, err
 	}
 
 	user := &model.User{
@@ -151,7 +150,7 @@ func (g *GitLab) Login(ctx context.Context, res http.ResponseWriter, req *http.R
 		user.Avatar = g.url + "/" + login.AvatarURL
 	}
 
-	return user, nil
+	return user, redirectURL, nil
 }
 
 // Refresh refreshes the Gitlab oauth2 access token. If the token is

server/forge/mocks/forge.go

@@ -242,34 +242,41 @@ func (_m *Forge) Hook(ctx context.Context, r *http.Request) (*model.Repo, *model
 	return r0, r1, r2
 }
 
-// Login provides a mock function with given fields: ctx, w, r
+// Login provides a mock function with given fields: ctx, r
-func (_m *Forge) Login(ctx context.Context, w http.ResponseWriter, r *http.Request) (*model.User, error) {
+func (_m *Forge) Login(ctx context.Context, r *types.OAuthRequest) (*model.User, string, error) {
-	ret := _m.Called(ctx, w, r)
+	ret := _m.Called(ctx, r)
 
 	if len(ret) == 0 {
 		panic("no return value specified for Login")
 	}
 
 	var r0 *model.User
-	var r1 error
+	var r1 string
-	if rf, ok := ret.Get(0).(func(context.Context, http.ResponseWriter, *http.Request) (*model.User, error)); ok {
+	var r2 error
-		return rf(ctx, w, r)
+	if rf, ok := ret.Get(0).(func(context.Context, *types.OAuthRequest) (*model.User, string, error)); ok {
+		return rf(ctx, r)
 	}
-	if rf, ok := ret.Get(0).(func(context.Context, http.ResponseWriter, *http.Request) *model.User); ok {
+	if rf, ok := ret.Get(0).(func(context.Context, *types.OAuthRequest) *model.User); ok {
-		r0 = rf(ctx, w, r)
+		r0 = rf(ctx, r)
 	} else {
 		if ret.Get(0) != nil {
 			r0 = ret.Get(0).(*model.User)
 		}
 	}
 
-	if rf, ok := ret.Get(1).(func(context.Context, http.ResponseWriter, *http.Request) error); ok {
+	if rf, ok := ret.Get(1).(func(context.Context, *types.OAuthRequest) string); ok {
-		r1 = rf(ctx, w, r)
+		r1 = rf(ctx, r)
 	} else {
-		r1 = ret.Error(1)
+		r1 = ret.Get(1).(string)
 	}
 
-	return r0, r1
+	if rf, ok := ret.Get(2).(func(context.Context, *types.OAuthRequest) error); ok {
+		r2 = rf(ctx, r)
+	} else {
+		r2 = ret.Error(2)
+	}
+
+	return r0, r1, r2
 }
 
 // Name provides a mock function with given fields:

server/forge/types/oauth.go

@@ -0,0 +1,22 @@
+// Copyright 2024 Woodpecker Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package types
+
+type OAuthRequest struct {
+	Error            string
+	ErrorURI         string
+	ErrorDescription string
+	Code             string
+}