From 56f709571184b1949b9bbf584eda259e3c4e04b5 Mon Sep 17 00:00:00 2001 From: Anbraten <6918444+anbraten@users.noreply.github.com> Date: Mon, 22 Apr 2024 15:47:57 +0200 Subject: [PATCH 1/8] login with selected forge --- server/api/login.go | 44 +++++++++++++++++------ server/services/manager.go | 21 +++++------ web/src/assets/locales/en.json | 2 +- web/src/compositions/useAuthentication.ts | 4 +-- web/src/views/Login.vue | 38 ++++++++++++++++++-- 5 files changed, 82 insertions(+), 27 deletions(-) diff --git a/server/api/login.go b/server/api/login.go index 349861ce11..fc3c228b25 100644 --- a/server/api/login.go +++ b/server/api/login.go @@ -17,7 +17,9 @@ package api import ( "encoding/base32" "errors" + "fmt" "net/http" + "strconv" "time" "github.com/gin-gonic/gin" @@ -33,22 +35,42 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/shared/token" ) +func getForgeID(c *gin.Context) int64 { + _forgeID := c.Query("forge_id") + if _forgeID == "" { + _forgeID, _ = c.Cookie("forge_id") + } + if _forgeID == "" { + return 1 + } + forgeID, err := strconv.ParseInt(_forgeID, 10, 64) + if err != nil { + return 1 + } + return forgeID +} + func HandleLogin(c *gin.Context) { + forgeID := getForgeID(c) + if err := c.Request.FormValue("error"); err != "" { - c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login/error?code="+err) + c.Redirect(http.StatusSeeOther, fmt.Sprintf("%s/login/error?code=%s&forge_id=%d", server.Config.Server.RootPath, err, forgeID)) } else { - c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/authorize") + c.Redirect(http.StatusSeeOther, fmt.Sprintf("%s/authorize?forge_id=%d", server.Config.Server.RootPath, forgeID)) } } func HandleAuth(c *gin.Context) { _store := store.FromContext(c) - _forge, err := server.Config.Services.Manager.ForgeMain() + forgeID := getForgeID(c) + + log.Debug().Msgf("trying to login with forge_id: %d", forgeID) + + _forge, err := server.Config.Services.Manager.ForgeByID(forgeID) if err != nil { _ = c.AbortWithError(http.StatusInternalServerError, err) return } - forgeID := int64(1) // TODO: replace with forge id when multiple forges are supported // when dealing with redirects, we may need to adjust the content type. I // cannot, however, remember why, so need to revisit this line. @@ -138,7 +160,7 @@ func HandleAuth(c *gin.Context) { ForgeID: u.ForgeID, } if err := _store.OrgCreate(org); err != nil { - log.Error().Err(err).Msgf("on user creation, could create org for user") + log.Error().Err(err).Msgf("on user creation, could not create org for user") } u.OrgID = org.ID } @@ -235,17 +257,16 @@ func GetLogout(c *gin.Context) { func GetLoginToken(c *gin.Context) { _store := store.FromContext(c) - _forge, err := server.Config.Services.Manager.ForgeMain() // TODO: get selected forge from auth request + in := &tokenPayload{} + err := c.Bind(in) if err != nil { - log.Error().Err(err).Msg("Cannot get main forge") - c.AbortWithStatus(http.StatusInternalServerError) + _ = c.AbortWithError(http.StatusBadRequest, err) return } - in := &tokenPayload{} - err = c.Bind(in) + _forge, err := server.Config.Services.Manager.ForgeByID(in.ForgeID) if err != nil { - _ = c.AbortWithError(http.StatusBadRequest, err) + _ = c.AbortWithError(http.StatusInternalServerError, err) return } @@ -279,4 +300,5 @@ type tokenPayload struct { Access string `json:"access_token,omitempty"` Refresh string `json:"refresh_token,omitempty"` Expires int64 `json:"expires_in,omitempty"` + ForgeID int64 `json:"forge_id,omitempty"` } diff --git a/server/services/manager.go b/server/services/manager.go index ff281c40a2..26859e58ac 100644 --- a/server/services/manager.go +++ b/server/services/manager.go @@ -46,6 +46,7 @@ type Manager interface { EnvironmentService() environment.Service ForgeFromRepo(repo *model.Repo) (forge.Forge, error) ForgeFromUser(user *model.User) (forge.Forge, error) + ForgeByID(forgeID int64) (forge.Forge, error) ForgeMain() (forge.Forge, error) } @@ -115,24 +116,20 @@ func (m *manager) EnvironmentService() environment.Service { } func (m *manager) ForgeFromRepo(repo *model.Repo) (forge.Forge, error) { - return m.getForgeByID(repo.ForgeID) + return m.ForgeByID(repo.ForgeID) } func (m *manager) ForgeFromUser(user *model.User) (forge.Forge, error) { - return m.getForgeByID(user.ForgeID) + return m.ForgeByID(user.ForgeID) } -func (m *manager) ForgeMain() (forge.Forge, error) { - return m.getForgeByID(1) // main forge is always 1 and is configured via environment variables -} - -func (m *manager) getForgeByID(id int64) (forge.Forge, error) { - item := m.forgeCache.Get(id) +func (m *manager) ForgeByID(forgeID int64) (forge.Forge, error) { + item := m.forgeCache.Get(forgeID) if item != nil && !item.IsExpired() { return item.Value(), nil } - forgeModel, err := m.store.ForgeGet(id) + forgeModel, err := m.store.ForgeGet(forgeID) if err != nil { return nil, err } @@ -142,7 +139,11 @@ func (m *manager) getForgeByID(id int64) (forge.Forge, error) { return nil, err } - m.forgeCache.Set(id, forge, forgeCacheTTL) + m.forgeCache.Set(forgeID, forge, forgeCacheTTL) return forge, nil } + +func (m *manager) ForgeMain() (forge.Forge, error) { + return m.ForgeByID(1) // main forge is always 1 and is configured via environment variables +} diff --git a/web/src/assets/locales/en.json b/web/src/assets/locales/en.json index 83e98c2d81..008b6b9e4a 100644 --- a/web/src/assets/locales/en.json +++ b/web/src/assets/locales/en.json @@ -1,6 +1,6 @@ { "cancel": "Cancel", - "login": "Login", + "login_with": "Login with {forge}", "welcome": "Welcome to Woodpecker", "repos": "Repos", "repositories": "Repositories", diff --git a/web/src/compositions/useAuthentication.ts b/web/src/compositions/useAuthentication.ts index 287942eed5..8ed98f42da 100644 --- a/web/src/compositions/useAuthentication.ts +++ b/web/src/compositions/useAuthentication.ts @@ -7,11 +7,11 @@ export default () => user: useConfig().user, - authenticate(url?: string) { + authenticate(url?: string, forgeId?: number) { if (url) { const config = useUserConfig(); config.setUserConfig('redirectUrl', url); } - window.location.href = `${useConfig().rootPath}/login`; + window.location.href = `${useConfig().rootPath}/login?forge_id=${forgeId}`; }, }) as const; diff --git a/web/src/views/Login.vue b/web/src/views/Login.vue index d6fcec0b98..dbb8c8e572 100644 --- a/web/src/views/Login.vue +++ b/web/src/views/Login.vue @@ -11,7 +11,11 @@

{{ $t('welcome') }}

- +
+ +
@@ -32,9 +36,37 @@ const authentication = useAuthentication(); const errorMessage = ref(); const i18n = useI18n(); -function doLogin() { +type Forge = { + id: number; + url: string; + type: string; +}; + +const forges = ref([ + { + id: 1, + url: 'http://localhost:3000/', + type: 'gitea', + }, + { + id: 2, + url: '', + type: 'github', + }, +]); + +function getHostFromUrl(forge: Forge) { + if (!forge.url) { + return forge.type.charAt(0).toUpperCase() + forge.type.slice(1); + } + + const url = new URL(forge.url); + return url.hostname; +} + +function doLogin(forgeId?: number) { const url = typeof route.query.url === 'string' ? route.query.url : ''; - authentication.authenticate(url); + authentication.authenticate(url, forgeId); } const authErrorMessages = { From 29f8a31730aa716e5fb6c122b3865017ca52faee Mon Sep 17 00:00:00 2001 From: Anbraten <6918444+anbraten@users.noreply.github.com> Date: Tue, 23 Apr 2024 10:41:37 +0200 Subject: [PATCH 2/8] allow to select forge on login --- server/api/login.go | 24 ++++++++++++++----- server/api/repo.go | 8 +++---- server/api/user.go | 4 ++-- server/api/users.go | 20 +++++++++++++--- server/forge/bitbucket/bitbucket.go | 2 +- .../bitbucketdatacenter.go | 2 +- server/forge/gitea/gitea.go | 2 +- server/forge/github/github.go | 2 +- server/forge/gitlab/gitlab.go | 2 +- server/forge/types/oauth.go | 1 + server/model/user.go | 4 ++-- server/router/middleware/session/user.go | 6 ++++- server/store/datastore/user.go | 18 ++++---------- server/store/store.go | 6 ++--- server/web/config.go | 3 ++- web/src/router.ts | 1 + 16 files changed, 65 insertions(+), 40 deletions(-) diff --git a/server/api/login.go b/server/api/login.go index fc3c228b25..03ae89b8e5 100644 --- a/server/api/login.go +++ b/server/api/login.go @@ -62,6 +62,8 @@ func HandleLogin(c *gin.Context) { func HandleAuth(c *gin.Context) { _store := store.FromContext(c) + + // TODO: get forge-id in state forgeID := getForgeID(c) log.Debug().Msgf("trying to login with forge_id: %d", forgeID) @@ -76,7 +78,11 @@ func HandleAuth(c *gin.Context) { // cannot, however, remember why, so need to revisit this line. c.Writer.Header().Del("Content-Type") + stateID := "woodpecker-state-123" // TODO: generate a random id + // TODO: add selected forgeID to content of the state + tmpuser, redirectURL, err := _forge.Login(c, &forge_types.OAuthRequest{ + State: stateID, Error: c.Request.FormValue("error"), ErrorURI: c.Request.FormValue("error_uri"), ErrorDescription: c.Request.FormValue("error_description"), @@ -87,14 +93,19 @@ func HandleAuth(c *gin.Context) { c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=oauth_error") return } + // The user is not authorized yet -> redirect if tmpuser == nil { + // TODO: set forge-id in state + httputil.SetCookie(c.Writer, c.Request, "forge_id", fmt.Sprintf("%d", forgeID)) http.Redirect(c.Writer, c.Request, redirectURL, http.StatusSeeOther) return } + // TODO: set forge-id in state + httputil.DelCookie(c.Writer, c.Request, "forge_id") // get the user from the database - u, err := _store.GetUserRemoteID(tmpuser.ForgeRemoteID, tmpuser.Login) + u, err := _store.GetUserRemoteID(forgeID, tmpuser.ForgeRemoteID) if err != nil && !errors.Is(err, types.RecordNotExist) { _ = c.AbortWithError(http.StatusInternalServerError, err) return @@ -207,7 +218,7 @@ func HandleAuth(c *gin.Context) { } exp := time.Now().Add(server.Config.Server.SessionExpires).Unix() - tokenString, err := token.New(token.SessToken, u.Login).SignExpires(u.Hash, exp) + tokenString, err := token.New(token.SessToken, strconv.FormatInt(u.ID, 10)).SignExpires(u.Hash, exp) if err != nil { log.Error().Msgf("cannot create token for %s", u.Login) c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=internal_error") @@ -264,7 +275,9 @@ func GetLoginToken(c *gin.Context) { return } - _forge, err := server.Config.Services.Manager.ForgeByID(in.ForgeID) + forgeID := int64(1) // TODO: use correct forge + + _forge, err := server.Config.Services.Manager.ForgeByID(forgeID) if err != nil { _ = c.AbortWithError(http.StatusInternalServerError, err) return @@ -276,14 +289,14 @@ func GetLoginToken(c *gin.Context) { return } - user, err := _store.GetUserLogin(login) + user, err := _store.GetUserLogin(forgeID, login) if err != nil { handleDBError(c, err) return } exp := time.Now().Add(server.Config.Server.SessionExpires).Unix() - newToken := token.New(token.SessToken, user.Login) + newToken := token.New(token.SessToken, strconv.FormatInt(user.ID, 10)) tokenStr, err := newToken.SignExpires(user.Hash, exp) if err != nil { _ = c.AbortWithError(http.StatusInternalServerError, err) @@ -300,5 +313,4 @@ type tokenPayload struct { Access string `json:"access_token,omitempty"` Refresh string `json:"refresh_token,omitempty"` Expires int64 `json:"expires_in,omitempty"` - ForgeID int64 `json:"forge_id,omitempty"` } diff --git a/server/api/repo.go b/server/api/repo.go index 6d03dcede3..77bb890cc2 100644 --- a/server/api/repo.go +++ b/server/api/repo.go @@ -118,7 +118,7 @@ func PostRepo(c *gin.Context) { } // creates the jwt token used to verify the repository - t := token.New(token.HookToken, repo.FullName) + t := token.New(token.HookToken, strconv.FormatInt(repo.ID, 10)) sig, err := t.Sign(repo.Hash) if err != nil { msg := "could not generate new jwt token." @@ -520,7 +520,7 @@ func MoveRepo(c *gin.Context) { } // creates the jwt token used to verify the repository - t := token.New(token.HookToken, repo.FullName) + t := token.New(token.HookToken, strconv.FormatInt(repo.ID, 10)) sig, err := t.Sign(repo.Hash) if err != nil { c.String(http.StatusInternalServerError, err.Error()) @@ -536,7 +536,7 @@ func MoveRepo(c *gin.Context) { ) if err := _forge.Deactivate(c, user, repo, host); err != nil { - log.Trace().Err(err).Msgf("deactivate repo '%s' for move to activate later, got an error", repo.FullName) + log.Trace().Err(err).Msgf("deactivate repo '%s' for move to activate later, got an error", strconv.FormatInt(repo.ID, 10)) } if err := _forge.Activate(c, user, repo, hookURL); err != nil { c.String(http.StatusInternalServerError, err.Error()) @@ -620,7 +620,7 @@ func repairRepo(c *gin.Context, repo *model.Repo, withPerms, skipOnErr bool) { } // creates the jwt token used to verify the repository - t := token.New(token.HookToken, repo.FullName) + t := token.New(token.HookToken, strconv.FormatInt(repo.ID, 10)) sig, err := t.Sign(repo.Hash) if err != nil { c.String(http.StatusInternalServerError, err.Error()) diff --git a/server/api/user.go b/server/api/user.go index d24849bf39..8cab2bbf1a 100644 --- a/server/api/user.go +++ b/server/api/user.go @@ -153,7 +153,7 @@ func GetRepos(c *gin.Context) { // @Param Authorization header string true "Insert your personal access token" default(Bearer ) func PostToken(c *gin.Context) { user := session.User(c) - tokenString, err := token.New(token.UserToken, user.Login).Sign(user.Hash) + tokenString, err := token.New(token.UserToken, strconv.FormatInt(user.ID, 10)).Sign(user.Hash) if err != nil { _ = c.AbortWithError(http.StatusInternalServerError, err) return @@ -182,7 +182,7 @@ func DeleteToken(c *gin.Context) { return } - tokenString, err := token.New(token.UserToken, user.Login).Sign(user.Hash) + tokenString, err := token.New(token.UserToken, strconv.FormatInt(user.ID, 10)).Sign(user.Hash) if err != nil { _ = c.AbortWithError(http.StatusInternalServerError, err) return diff --git a/server/api/users.go b/server/api/users.go index c35339eefa..59faf04df1 100644 --- a/server/api/users.go +++ b/server/api/users.go @@ -17,6 +17,7 @@ package api import ( "encoding/base32" "net/http" + "strconv" "github.com/gin-gonic/gin" "github.com/gorilla/securecookie" @@ -57,7 +58,8 @@ func GetUsers(c *gin.Context) { // @Param Authorization header string true "Insert your personal access token" default(Bearer ) // @Param login path string true "the user's login name" func GetUser(c *gin.Context) { - user, err := store.FromContext(c).GetUserLogin(c.Param("login")) + forgeID := int64(1) // TODO: use correct forge + user, err := getUserFromLogin(c, forgeID) if err != nil { handleDBError(c, err) return @@ -87,7 +89,8 @@ func PatchUser(c *gin.Context) { return } - user, err := _store.GetUserLogin(c.Param("login")) + forgeID := int64(1) // TODO: use correct forge + user, err := getUserFromLogin(c, forgeID) if err != nil { handleDBError(c, err) return @@ -159,7 +162,8 @@ func PostUser(c *gin.Context) { func DeleteUser(c *gin.Context) { _store := store.FromContext(c) - user, err := _store.GetUserLogin(c.Param("login")) + forgeID := int64(1) // TODO: use correct forge + user, err := getUserFromLogin(c, forgeID) if err != nil { handleDBError(c, err) return @@ -170,3 +174,13 @@ func DeleteUser(c *gin.Context) { } c.Status(http.StatusNoContent) } + +func getUserFromLogin(c *gin.Context, forgeID int64) (*model.User, error) { + _store := store.FromContext(c) + + if userID, err := strconv.ParseInt(c.Param("login"), 10, 64); err == nil { + return _store.GetUser(userID) + } + + return _store.GetUserLogin(forgeID, c.Param("login")) +} diff --git a/server/forge/bitbucket/bitbucket.go b/server/forge/bitbucket/bitbucket.go index d3c96920e6..05d5c7dae6 100644 --- a/server/forge/bitbucket/bitbucket.go +++ b/server/forge/bitbucket/bitbucket.go @@ -81,7 +81,7 @@ func (c *config) URL() string { // Bitbucket account details are returned when the user is successfully authenticated. func (c *config) Login(ctx context.Context, req *forge_types.OAuthRequest) (*model.User, string, error) { config := c.newOAuth2Config() - redirectURL := config.AuthCodeURL("woodpecker") + redirectURL := config.AuthCodeURL(req.State) // get the OAuth errors if req.Error != "" { diff --git a/server/forge/bitbucketdatacenter/bitbucketdatacenter.go b/server/forge/bitbucketdatacenter/bitbucketdatacenter.go index b881a701db..e9c2234a5a 100644 --- a/server/forge/bitbucketdatacenter/bitbucketdatacenter.go +++ b/server/forge/bitbucketdatacenter/bitbucketdatacenter.go @@ -94,7 +94,7 @@ func (c *client) Login(ctx context.Context, req *forge_types.OAuthRequest) (*mod config := c.newOAuth2Config() // TODO: Add proper state and pkce... - redirectURL := config.AuthCodeURL("woodpecker") + redirectURL := config.AuthCodeURL(req.State) if req.Error != "" { return nil, redirectURL, &forge_types.AuthError{ diff --git a/server/forge/gitea/gitea.go b/server/forge/gitea/gitea.go index 964acb1cb9..4bfbdfc65f 100644 --- a/server/forge/gitea/gitea.go +++ b/server/forge/gitea/gitea.go @@ -115,7 +115,7 @@ func (c *Gitea) oauth2Config(ctx context.Context) (*oauth2.Config, context.Conte // Gitea account details are returned when the user is successfully authenticated. func (c *Gitea) Login(ctx context.Context, req *forge_types.OAuthRequest) (*model.User, string, error) { config, oauth2Ctx := c.oauth2Config(ctx) - redirectURL := config.AuthCodeURL("woodpecker") + redirectURL := config.AuthCodeURL(req.State) // check the OAuth errors if req.Error != "" { diff --git a/server/forge/github/github.go b/server/forge/github/github.go index 3e0ae25dc5..7102bcb57e 100644 --- a/server/forge/github/github.go +++ b/server/forge/github/github.go @@ -97,7 +97,7 @@ func (c *client) URL() string { // Login authenticates the session and returns the forge user details. func (c *client) Login(ctx context.Context, req *forge_types.OAuthRequest) (*model.User, string, error) { config := c.newConfig() - redirectURL := config.AuthCodeURL("woodpecker") + redirectURL := config.AuthCodeURL(req.State) // check the OAuth errors if req.Error != "" { diff --git a/server/forge/gitlab/gitlab.go b/server/forge/gitlab/gitlab.go index 4f05659a59..a80bee7d22 100644 --- a/server/forge/gitlab/gitlab.go +++ b/server/forge/gitlab/gitlab.go @@ -107,7 +107,7 @@ func (g *GitLab) oauth2Config(ctx context.Context) (*oauth2.Config, context.Cont // forge user details. func (g *GitLab) Login(ctx context.Context, req *forge_types.OAuthRequest) (*model.User, string, error) { config, oauth2Ctx := g.oauth2Config(ctx) - redirectURL := config.AuthCodeURL("woodpecker") + redirectURL := config.AuthCodeURL(req.State) // check the OAuth errors if req.Error != "" { diff --git a/server/forge/types/oauth.go b/server/forge/types/oauth.go index fba870569c..0897d7aa9b 100644 --- a/server/forge/types/oauth.go +++ b/server/forge/types/oauth.go @@ -15,6 +15,7 @@ package types type OAuthRequest struct { + State string Error string ErrorURI string ErrorDescription string diff --git a/server/model/user.go b/server/model/user.go index 578d6caf28..9323d024dc 100644 --- a/server/model/user.go +++ b/server/model/user.go @@ -34,14 +34,14 @@ type User struct { // required: true ID int64 `json:"id" xorm:"pk autoincr 'user_id'"` - ForgeID int64 `json:"forge_id,omitempty" xorm:"forge_id"` + ForgeID int64 `json:"forge_id,omitempty" xorm:"UNIQUE(s) 'forge_id'"` ForgeRemoteID ForgeRemoteID `json:"-" xorm:"forge_remote_id"` // Login is the username for this user. // // required: true - Login string `json:"login" xorm:"UNIQUE 'user_login'"` + Login string `json:"login" xorm:"UNIQUE(s) 'user_login'"` // Token is the oauth2 token. Token string `json:"-" xorm:"TEXT 'user_token'"` diff --git a/server/router/middleware/session/user.go b/server/router/middleware/session/user.go index d1b1507ef7..1b96b6864e 100644 --- a/server/router/middleware/session/user.go +++ b/server/router/middleware/session/user.go @@ -45,7 +45,11 @@ func SetUser() gin.HandlerFunc { t, err := token.ParseRequest(c.Request, func(t *token.Token) (string, error) { var err error - user, err = store.FromContext(c).GetUserLogin(t.Text) + userID, err := strconv.ParseInt(t.Text, 10, 64) + if err != nil { + return "", err + } + user, err = store.FromContext(c).GetUser(userID) return user.Hash, err }) if err == nil { diff --git a/server/store/datastore/user.go b/server/store/datastore/user.go index 39f09af75e..81b461623f 100644 --- a/server/store/datastore/user.go +++ b/server/store/datastore/user.go @@ -15,8 +15,6 @@ package datastore import ( - "xorm.io/xorm" - "go.woodpecker-ci.org/woodpecker/v2/server/model" ) @@ -25,23 +23,17 @@ func (s storage) GetUser(id int64) (*model.User, error) { return user, wrapGet(s.engine.ID(id).Get(user)) } -func (s storage) GetUserRemoteID(remoteID model.ForgeRemoteID, login string) (*model.User, error) { +func (s storage) GetUserRemoteID(forgeID int64, remoteID model.ForgeRemoteID) (*model.User, error) { sess := s.engine.NewSession() user := new(model.User) - err := wrapGet(sess.Where("forge_remote_id = ?", remoteID).Get(user)) - if err != nil { - user, err = s.getUserLogin(sess, login) - } + err := wrapGet(sess.Where("forge_id=? AND forge_remote_id = ?", forgeID, remoteID).Get(user)) return user, err } -func (s storage) GetUserLogin(login string) (*model.User, error) { - return s.getUserLogin(s.engine.NewSession(), login) -} - -func (s storage) getUserLogin(sess *xorm.Session, login string) (*model.User, error) { +func (s storage) GetUserLogin(forgeID int64, login string) (*model.User, error) { + sess := s.engine.NewSession() user := new(model.User) - return user, wrapGet(sess.Where("user_login=?", login).Get(user)) + return user, wrapGet(sess.Where("forge_id=? AND user_login=?", forgeID, login).Get(user)) } func (s storage) GetUserList(p *model.ListOptions) ([]*model.User, error) { diff --git a/server/store/store.go b/server/store/store.go index 6ba76573c4..7772e7c9c6 100644 --- a/server/store/store.go +++ b/server/store/store.go @@ -26,10 +26,10 @@ type Store interface { // Users // GetUser gets a user by unique ID. GetUser(int64) (*model.User, error) - // GetUserRemoteID gets a user by remote ID with fallback to login name. - GetUserRemoteID(model.ForgeRemoteID, string) (*model.User, error) + // GetUserRemoteID gets a user by remote ID. + GetUserRemoteID(forgeID int64, remoteID model.ForgeRemoteID) (*model.User, error) // GetUserLogin gets a user by unique Login name. - GetUserLogin(string) (*model.User, error) + GetUserLogin(forgeID int64, login string) (*model.User, error) // GetUserList gets a list of all users in the system. GetUserList(p *model.ListOptions) ([]*model.User, error) // GetUserCount gets a count of all users in the system. diff --git a/server/web/config.go b/server/web/config.go index 99e78461e0..987ef128d8 100644 --- a/server/web/config.go +++ b/server/web/config.go @@ -17,6 +17,7 @@ package web import ( "encoding/json" "net/http" + "strconv" "text/template" "github.com/gin-gonic/gin" @@ -35,7 +36,7 @@ func Config(c *gin.Context) { if user != nil { csrf, _ = token.New( token.CsrfToken, - user.Login, + strconv.FormatInt(user.ID, 10), ).Sign(user.Hash) } diff --git a/web/src/router.ts b/web/src/router.ts index e3540479c6..7053006ccc 100644 --- a/web/src/router.ts +++ b/web/src/router.ts @@ -202,6 +202,7 @@ router.beforeEach(async (to, _, next) => { if (redirectUrl !== '') { config.setUserConfig('redirectUrl', ''); next(redirectUrl); + return; } const authentication = useAuthentication(); From e1a1c246f4e11a460683f2ed09b219a245e21956 Mon Sep 17 00:00:00 2001 From: Anbraten <6918444+anbraten@users.noreply.github.com> Date: Sun, 12 May 2024 08:19:53 +0200 Subject: [PATCH 3/8] undo --- server/api/users.go | 20 ++-------- server/forge/bitbucket/bitbucket.go | 2 +- .../bitbucketdatacenter.go | 2 +- server/forge/gitea/gitea.go | 2 +- server/forge/github/github.go | 2 +- server/forge/gitlab/gitlab.go | 2 +- server/forge/types/oauth.go | 1 - server/model/user.go | 4 +- server/services/manager.go | 21 +++++----- server/store/datastore/user.go | 18 ++++++--- server/store/store.go | 6 +-- web/src/assets/locales/en.json | 2 +- web/src/compositions/useAuthentication.ts | 4 +- web/src/router.ts | 1 - web/src/views/Login.vue | 38 ++----------------- 15 files changed, 42 insertions(+), 83 deletions(-) diff --git a/server/api/users.go b/server/api/users.go index bd9829922b..1a591d1271 100644 --- a/server/api/users.go +++ b/server/api/users.go @@ -17,7 +17,6 @@ package api import ( "encoding/base32" "net/http" - "strconv" "github.com/gin-gonic/gin" "github.com/gorilla/securecookie" @@ -58,8 +57,7 @@ func GetUsers(c *gin.Context) { // @Param Authorization header string true "Insert your personal access token" default(Bearer ) // @Param login path string true "the user's login name" func GetUser(c *gin.Context) { - forgeID := int64(1) // TODO: use correct forge - user, err := getUserFromLogin(c, forgeID) + user, err := store.FromContext(c).GetUserLogin(c.Param("login")) if err != nil { handleDBError(c, err) return @@ -89,8 +87,7 @@ func PatchUser(c *gin.Context) { return } - forgeID := int64(1) // TODO: use correct forge - user, err := getUserFromLogin(c, forgeID) + user, err := _store.GetUserLogin(c.Param("login")) if err != nil { handleDBError(c, err) return @@ -162,8 +159,7 @@ func PostUser(c *gin.Context) { func DeleteUser(c *gin.Context) { _store := store.FromContext(c) - forgeID := int64(1) // TODO: use correct forge - user, err := getUserFromLogin(c, forgeID) + user, err := _store.GetUserLogin(c.Param("login")) if err != nil { handleDBError(c, err) return @@ -174,13 +170,3 @@ func DeleteUser(c *gin.Context) { } c.Status(http.StatusNoContent) } - -func getUserFromLogin(c *gin.Context, forgeID int64) (*model.User, error) { - _store := store.FromContext(c) - - if userID, err := strconv.ParseInt(c.Param("login"), 10, 64); err == nil { - return _store.GetUser(userID) - } - - return _store.GetUserLogin(forgeID, c.Param("login")) -} diff --git a/server/forge/bitbucket/bitbucket.go b/server/forge/bitbucket/bitbucket.go index 05d5c7dae6..d3c96920e6 100644 --- a/server/forge/bitbucket/bitbucket.go +++ b/server/forge/bitbucket/bitbucket.go @@ -81,7 +81,7 @@ func (c *config) URL() string { // Bitbucket account details are returned when the user is successfully authenticated. func (c *config) Login(ctx context.Context, req *forge_types.OAuthRequest) (*model.User, string, error) { config := c.newOAuth2Config() - redirectURL := config.AuthCodeURL(req.State) + redirectURL := config.AuthCodeURL("woodpecker") // get the OAuth errors if req.Error != "" { diff --git a/server/forge/bitbucketdatacenter/bitbucketdatacenter.go b/server/forge/bitbucketdatacenter/bitbucketdatacenter.go index 312b90d923..58b0c6d372 100644 --- a/server/forge/bitbucketdatacenter/bitbucketdatacenter.go +++ b/server/forge/bitbucketdatacenter/bitbucketdatacenter.go @@ -94,7 +94,7 @@ func (c *client) Login(ctx context.Context, req *forge_types.OAuthRequest) (*mod config := c.newOAuth2Config() // TODO: Add proper state and pkce... - redirectURL := config.AuthCodeURL(req.State) + redirectURL := config.AuthCodeURL("woodpecker") if req.Error != "" { return nil, redirectURL, &forge_types.AuthError{ diff --git a/server/forge/gitea/gitea.go b/server/forge/gitea/gitea.go index 4bfbdfc65f..964acb1cb9 100644 --- a/server/forge/gitea/gitea.go +++ b/server/forge/gitea/gitea.go @@ -115,7 +115,7 @@ func (c *Gitea) oauth2Config(ctx context.Context) (*oauth2.Config, context.Conte // Gitea account details are returned when the user is successfully authenticated. func (c *Gitea) Login(ctx context.Context, req *forge_types.OAuthRequest) (*model.User, string, error) { config, oauth2Ctx := c.oauth2Config(ctx) - redirectURL := config.AuthCodeURL(req.State) + redirectURL := config.AuthCodeURL("woodpecker") // check the OAuth errors if req.Error != "" { diff --git a/server/forge/github/github.go b/server/forge/github/github.go index 7102bcb57e..3e0ae25dc5 100644 --- a/server/forge/github/github.go +++ b/server/forge/github/github.go @@ -97,7 +97,7 @@ func (c *client) URL() string { // Login authenticates the session and returns the forge user details. func (c *client) Login(ctx context.Context, req *forge_types.OAuthRequest) (*model.User, string, error) { config := c.newConfig() - redirectURL := config.AuthCodeURL(req.State) + redirectURL := config.AuthCodeURL("woodpecker") // check the OAuth errors if req.Error != "" { diff --git a/server/forge/gitlab/gitlab.go b/server/forge/gitlab/gitlab.go index a80bee7d22..4f05659a59 100644 --- a/server/forge/gitlab/gitlab.go +++ b/server/forge/gitlab/gitlab.go @@ -107,7 +107,7 @@ func (g *GitLab) oauth2Config(ctx context.Context) (*oauth2.Config, context.Cont // forge user details. func (g *GitLab) Login(ctx context.Context, req *forge_types.OAuthRequest) (*model.User, string, error) { config, oauth2Ctx := g.oauth2Config(ctx) - redirectURL := config.AuthCodeURL(req.State) + redirectURL := config.AuthCodeURL("woodpecker") // check the OAuth errors if req.Error != "" { diff --git a/server/forge/types/oauth.go b/server/forge/types/oauth.go index 0897d7aa9b..fba870569c 100644 --- a/server/forge/types/oauth.go +++ b/server/forge/types/oauth.go @@ -15,7 +15,6 @@ package types type OAuthRequest struct { - State string Error string ErrorURI string ErrorDescription string diff --git a/server/model/user.go b/server/model/user.go index 9323d024dc..578d6caf28 100644 --- a/server/model/user.go +++ b/server/model/user.go @@ -34,14 +34,14 @@ type User struct { // required: true ID int64 `json:"id" xorm:"pk autoincr 'user_id'"` - ForgeID int64 `json:"forge_id,omitempty" xorm:"UNIQUE(s) 'forge_id'"` + ForgeID int64 `json:"forge_id,omitempty" xorm:"forge_id"` ForgeRemoteID ForgeRemoteID `json:"-" xorm:"forge_remote_id"` // Login is the username for this user. // // required: true - Login string `json:"login" xorm:"UNIQUE(s) 'user_login'"` + Login string `json:"login" xorm:"UNIQUE 'user_login'"` // Token is the oauth2 token. Token string `json:"-" xorm:"TEXT 'user_token'"` diff --git a/server/services/manager.go b/server/services/manager.go index 26859e58ac..ff281c40a2 100644 --- a/server/services/manager.go +++ b/server/services/manager.go @@ -46,7 +46,6 @@ type Manager interface { EnvironmentService() environment.Service ForgeFromRepo(repo *model.Repo) (forge.Forge, error) ForgeFromUser(user *model.User) (forge.Forge, error) - ForgeByID(forgeID int64) (forge.Forge, error) ForgeMain() (forge.Forge, error) } @@ -116,20 +115,24 @@ func (m *manager) EnvironmentService() environment.Service { } func (m *manager) ForgeFromRepo(repo *model.Repo) (forge.Forge, error) { - return m.ForgeByID(repo.ForgeID) + return m.getForgeByID(repo.ForgeID) } func (m *manager) ForgeFromUser(user *model.User) (forge.Forge, error) { - return m.ForgeByID(user.ForgeID) + return m.getForgeByID(user.ForgeID) } -func (m *manager) ForgeByID(forgeID int64) (forge.Forge, error) { - item := m.forgeCache.Get(forgeID) +func (m *manager) ForgeMain() (forge.Forge, error) { + return m.getForgeByID(1) // main forge is always 1 and is configured via environment variables +} + +func (m *manager) getForgeByID(id int64) (forge.Forge, error) { + item := m.forgeCache.Get(id) if item != nil && !item.IsExpired() { return item.Value(), nil } - forgeModel, err := m.store.ForgeGet(forgeID) + forgeModel, err := m.store.ForgeGet(id) if err != nil { return nil, err } @@ -139,11 +142,7 @@ func (m *manager) ForgeByID(forgeID int64) (forge.Forge, error) { return nil, err } - m.forgeCache.Set(forgeID, forge, forgeCacheTTL) + m.forgeCache.Set(id, forge, forgeCacheTTL) return forge, nil } - -func (m *manager) ForgeMain() (forge.Forge, error) { - return m.ForgeByID(1) // main forge is always 1 and is configured via environment variables -} diff --git a/server/store/datastore/user.go b/server/store/datastore/user.go index 81b461623f..39f09af75e 100644 --- a/server/store/datastore/user.go +++ b/server/store/datastore/user.go @@ -15,6 +15,8 @@ package datastore import ( + "xorm.io/xorm" + "go.woodpecker-ci.org/woodpecker/v2/server/model" ) @@ -23,17 +25,23 @@ func (s storage) GetUser(id int64) (*model.User, error) { return user, wrapGet(s.engine.ID(id).Get(user)) } -func (s storage) GetUserRemoteID(forgeID int64, remoteID model.ForgeRemoteID) (*model.User, error) { +func (s storage) GetUserRemoteID(remoteID model.ForgeRemoteID, login string) (*model.User, error) { sess := s.engine.NewSession() user := new(model.User) - err := wrapGet(sess.Where("forge_id=? AND forge_remote_id = ?", forgeID, remoteID).Get(user)) + err := wrapGet(sess.Where("forge_remote_id = ?", remoteID).Get(user)) + if err != nil { + user, err = s.getUserLogin(sess, login) + } return user, err } -func (s storage) GetUserLogin(forgeID int64, login string) (*model.User, error) { - sess := s.engine.NewSession() +func (s storage) GetUserLogin(login string) (*model.User, error) { + return s.getUserLogin(s.engine.NewSession(), login) +} + +func (s storage) getUserLogin(sess *xorm.Session, login string) (*model.User, error) { user := new(model.User) - return user, wrapGet(sess.Where("forge_id=? AND user_login=?", forgeID, login).Get(user)) + return user, wrapGet(sess.Where("user_login=?", login).Get(user)) } func (s storage) GetUserList(p *model.ListOptions) ([]*model.User, error) { diff --git a/server/store/store.go b/server/store/store.go index acc7bd71ef..2a007e487c 100644 --- a/server/store/store.go +++ b/server/store/store.go @@ -26,10 +26,10 @@ type Store interface { // Users // GetUser gets a user by unique ID. GetUser(int64) (*model.User, error) - // GetUserRemoteID gets a user by remote ID. - GetUserRemoteID(forgeID int64, remoteID model.ForgeRemoteID) (*model.User, error) + // GetUserRemoteID gets a user by remote ID with fallback to login name. + GetUserRemoteID(model.ForgeRemoteID, string) (*model.User, error) // GetUserLogin gets a user by unique Login name. - GetUserLogin(forgeID int64, login string) (*model.User, error) + GetUserLogin(string) (*model.User, error) // GetUserList gets a list of all users in the system. GetUserList(p *model.ListOptions) ([]*model.User, error) // GetUserCount gets a count of all users in the system. diff --git a/web/src/assets/locales/en.json b/web/src/assets/locales/en.json index 94f5380195..ed881f88fc 100644 --- a/web/src/assets/locales/en.json +++ b/web/src/assets/locales/en.json @@ -1,6 +1,6 @@ { "cancel": "Cancel", - "login_with": "Login with {forge}", + "login": "Login", "welcome": "Welcome to Woodpecker", "repos": "Repos", "repositories": "Repositories", diff --git a/web/src/compositions/useAuthentication.ts b/web/src/compositions/useAuthentication.ts index 8ed98f42da..287942eed5 100644 --- a/web/src/compositions/useAuthentication.ts +++ b/web/src/compositions/useAuthentication.ts @@ -7,11 +7,11 @@ export default () => user: useConfig().user, - authenticate(url?: string, forgeId?: number) { + authenticate(url?: string) { if (url) { const config = useUserConfig(); config.setUserConfig('redirectUrl', url); } - window.location.href = `${useConfig().rootPath}/login?forge_id=${forgeId}`; + window.location.href = `${useConfig().rootPath}/login`; }, }) as const; diff --git a/web/src/router.ts b/web/src/router.ts index 7053006ccc..e3540479c6 100644 --- a/web/src/router.ts +++ b/web/src/router.ts @@ -202,7 +202,6 @@ router.beforeEach(async (to, _, next) => { if (redirectUrl !== '') { config.setUserConfig('redirectUrl', ''); next(redirectUrl); - return; } const authentication = useAuthentication(); diff --git a/web/src/views/Login.vue b/web/src/views/Login.vue index dbb8c8e572..d6fcec0b98 100644 --- a/web/src/views/Login.vue +++ b/web/src/views/Login.vue @@ -11,11 +11,7 @@

{{ $t('welcome') }}

-
- -
+
@@ -36,37 +32,9 @@ const authentication = useAuthentication(); const errorMessage = ref(); const i18n = useI18n(); -type Forge = { - id: number; - url: string; - type: string; -}; - -const forges = ref([ - { - id: 1, - url: 'http://localhost:3000/', - type: 'gitea', - }, - { - id: 2, - url: '', - type: 'github', - }, -]); - -function getHostFromUrl(forge: Forge) { - if (!forge.url) { - return forge.type.charAt(0).toUpperCase() + forge.type.slice(1); - } - - const url = new URL(forge.url); - return url.hostname; -} - -function doLogin(forgeId?: number) { +function doLogin() { const url = typeof route.query.url === 'string' ? route.query.url : ''; - authentication.authenticate(url, forgeId); + authentication.authenticate(url); } const authErrorMessages = { From 5645696bb9db05ddfb5b2fbdf16035208684bb27 Mon Sep 17 00:00:00 2001 From: Anbraten <6918444+anbraten@users.noreply.github.com> Date: Sun, 12 May 2024 08:25:45 +0200 Subject: [PATCH 4/8] undo --- server/api/login.go | 57 ++++++++++----------------------------------- 1 file changed, 12 insertions(+), 45 deletions(-) diff --git a/server/api/login.go b/server/api/login.go index 03ae89b8e5..5cafc85bfd 100644 --- a/server/api/login.go +++ b/server/api/login.go @@ -17,7 +17,6 @@ package api import ( "encoding/base32" "errors" - "fmt" "net/http" "strconv" "time" @@ -35,54 +34,28 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/shared/token" ) -func getForgeID(c *gin.Context) int64 { - _forgeID := c.Query("forge_id") - if _forgeID == "" { - _forgeID, _ = c.Cookie("forge_id") - } - if _forgeID == "" { - return 1 - } - forgeID, err := strconv.ParseInt(_forgeID, 10, 64) - if err != nil { - return 1 - } - return forgeID -} - func HandleLogin(c *gin.Context) { - forgeID := getForgeID(c) - if err := c.Request.FormValue("error"); err != "" { - c.Redirect(http.StatusSeeOther, fmt.Sprintf("%s/login/error?code=%s&forge_id=%d", server.Config.Server.RootPath, err, forgeID)) + c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login/error?code="+err) } else { - c.Redirect(http.StatusSeeOther, fmt.Sprintf("%s/authorize?forge_id=%d", server.Config.Server.RootPath, forgeID)) + c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/authorize") } } func HandleAuth(c *gin.Context) { _store := store.FromContext(c) - - // TODO: get forge-id in state - forgeID := getForgeID(c) - - log.Debug().Msgf("trying to login with forge_id: %d", forgeID) - - _forge, err := server.Config.Services.Manager.ForgeByID(forgeID) + _forge, err := server.Config.Services.Manager.ForgeMain() if err != nil { _ = c.AbortWithError(http.StatusInternalServerError, err) return } + forgeID := int64(1) // TODO: replace with forge id when multiple forges are supported // when dealing with redirects, we may need to adjust the content type. I // cannot, however, remember why, so need to revisit this line. c.Writer.Header().Del("Content-Type") - stateID := "woodpecker-state-123" // TODO: generate a random id - // TODO: add selected forgeID to content of the state - tmpuser, redirectURL, err := _forge.Login(c, &forge_types.OAuthRequest{ - State: stateID, Error: c.Request.FormValue("error"), ErrorURI: c.Request.FormValue("error_uri"), ErrorDescription: c.Request.FormValue("error_description"), @@ -93,19 +66,14 @@ func HandleAuth(c *gin.Context) { c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=oauth_error") return } - // The user is not authorized yet -> redirect if tmpuser == nil { - // TODO: set forge-id in state - httputil.SetCookie(c.Writer, c.Request, "forge_id", fmt.Sprintf("%d", forgeID)) http.Redirect(c.Writer, c.Request, redirectURL, http.StatusSeeOther) return } - // TODO: set forge-id in state - httputil.DelCookie(c.Writer, c.Request, "forge_id") // get the user from the database - u, err := _store.GetUserRemoteID(forgeID, tmpuser.ForgeRemoteID) + u, err := _store.GetUserRemoteID(tmpuser.ForgeRemoteID, tmpuser.Login) if err != nil && !errors.Is(err, types.RecordNotExist) { _ = c.AbortWithError(http.StatusInternalServerError, err) return @@ -268,18 +236,17 @@ func GetLogout(c *gin.Context) { func GetLoginToken(c *gin.Context) { _store := store.FromContext(c) - in := &tokenPayload{} - err := c.Bind(in) + _forge, err := server.Config.Services.Manager.ForgeMain() // TODO: get selected forge from auth request if err != nil { - _ = c.AbortWithError(http.StatusBadRequest, err) + log.Error().Err(err).Msg("Cannot get main forge") + c.AbortWithStatus(http.StatusInternalServerError) return } - forgeID := int64(1) // TODO: use correct forge - - _forge, err := server.Config.Services.Manager.ForgeByID(forgeID) + in := &tokenPayload{} + err = c.Bind(in) if err != nil { - _ = c.AbortWithError(http.StatusInternalServerError, err) + _ = c.AbortWithError(http.StatusBadRequest, err) return } @@ -289,7 +256,7 @@ func GetLoginToken(c *gin.Context) { return } - user, err := _store.GetUserLogin(forgeID, login) + user, err := _store.GetUserLogin(login) if err != nil { handleDBError(c, err) return From 7106c456d146af2f0bee96ca3ec0a7f406848f50 Mon Sep 17 00:00:00 2001 From: Anbraten <6918444+anbraten@users.noreply.github.com> Date: Tue, 14 May 2024 12:24:56 +0200 Subject: [PATCH 5/8] use explicit token claims --- server/api/hook.go | 11 ++++--- server/api/login.go | 7 +++-- server/api/repo.go | 9 ++++-- server/api/user.go | 8 +++-- server/router/middleware/session/user.go | 2 +- server/web/config.go | 7 ++--- shared/token/token.go | 39 +++++++++++++++++------- 7 files changed, 55 insertions(+), 28 deletions(-) diff --git a/server/api/hook.go b/server/api/hook.go index 8046d289f1..207cf714bf 100644 --- a/server/api/hook.go +++ b/server/api/hook.go @@ -22,6 +22,7 @@ import ( "errors" "fmt" "net/http" + "strconv" "github.com/gin-gonic/gin" "github.com/rs/zerolog/log" @@ -159,7 +160,7 @@ func PostHook(c *gin.Context) { c.Status(http.StatusNoContent) return } - oldFullName := repo.FullName + currentRepoFullName := repo.FullName if repo.UserID == 0 { log.Warn().Msgf("ignoring hook. repo %s has no owner.", repo.FullName) @@ -179,7 +180,7 @@ func PostHook(c *gin.Context) { // // get the token and verify the hook is authorized - parsed, err := token.ParseRequest(c.Request, func(_ *token.Token) (string, error) { + parsedToken, err := token.ParseRequest(c.Request, func(_ *token.Token) (string, error) { return repo.Hash, nil }) if err != nil { @@ -188,7 +189,7 @@ func PostHook(c *gin.Context) { c.String(http.StatusBadRequest, msg) return } - verifiedKey := parsed.Text == oldFullName + verifiedKey := parsedToken.Get("repo-id") == strconv.FormatInt(repo.ID, 10) || parsedToken.Get("text") == currentRepoFullName if !verifiedKey { verifiedKey, err = _store.HasRedirectionForRepo(repo.ID, repo.FullName) if err != nil { @@ -200,7 +201,7 @@ func PostHook(c *gin.Context) { } if !verifiedKey { - msg := fmt.Sprintf("failure to verify token from hook. Expected %s, got %s", repo.FullName, parsed.Text) + msg := fmt.Sprintf("failure to verify token from hook. Expected %s, got %s", repo.FullName, parsedToken.Get("text")) log.Debug().Msg(msg) c.String(http.StatusForbidden, msg) return @@ -210,7 +211,7 @@ func PostHook(c *gin.Context) { // 4. Update repo // - if oldFullName != tmpRepo.FullName { + if currentRepoFullName != tmpRepo.FullName { // create a redirection err = _store.CreateRedirection(&model.Redirection{RepoID: repo.ID, FullName: repo.FullName}) if err != nil { diff --git a/server/api/login.go b/server/api/login.go index 5cafc85bfd..bdd21dd730 100644 --- a/server/api/login.go +++ b/server/api/login.go @@ -186,7 +186,9 @@ func HandleAuth(c *gin.Context) { } exp := time.Now().Add(server.Config.Server.SessionExpires).Unix() - tokenString, err := token.New(token.SessToken, strconv.FormatInt(u.ID, 10)).SignExpires(u.Hash, exp) + _token := token.New(token.SessToken) + _token.Set("user-id", strconv.FormatInt(u.ID, 10)) + tokenString, err := _token.SignExpires(u.Hash, exp) if err != nil { log.Error().Msgf("cannot create token for %s", u.Login) c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=internal_error") @@ -263,7 +265,8 @@ func GetLoginToken(c *gin.Context) { } exp := time.Now().Add(server.Config.Server.SessionExpires).Unix() - newToken := token.New(token.SessToken, strconv.FormatInt(user.ID, 10)) + newToken := token.New(token.SessToken) + newToken.Set("user-id", strconv.FormatInt(user.ID, 10)) tokenStr, err := newToken.SignExpires(user.Hash, exp) if err != nil { _ = c.AbortWithError(http.StatusInternalServerError, err) diff --git a/server/api/repo.go b/server/api/repo.go index 4d80b8d45f..faca0444b7 100644 --- a/server/api/repo.go +++ b/server/api/repo.go @@ -118,7 +118,8 @@ func PostRepo(c *gin.Context) { } // creates the jwt token used to verify the repository - t := token.New(token.HookToken, strconv.FormatInt(repo.ID, 10)) + t := token.New(token.HookToken) + t.Set("repo-id", strconv.FormatInt(repo.ID, 10)) sig, err := t.Sign(repo.Hash) if err != nil { msg := "could not generate new jwt token." @@ -520,7 +521,8 @@ func MoveRepo(c *gin.Context) { } // creates the jwt token used to verify the repository - t := token.New(token.HookToken, strconv.FormatInt(repo.ID, 10)) + t := token.New(token.HookToken) + t.Set("repo-id", strconv.FormatInt(repo.ID, 10)) sig, err := t.Sign(repo.Hash) if err != nil { c.String(http.StatusInternalServerError, err.Error()) @@ -622,7 +624,8 @@ func repairRepo(c *gin.Context, repo *model.Repo, withPerms, skipOnErr bool) { } // creates the jwt token used to verify the repository - t := token.New(token.HookToken, strconv.FormatInt(repo.ID, 10)) + t := token.New(token.HookToken) + t.Set("repo-id", strconv.FormatInt(repo.ID, 10)) sig, err := t.Sign(repo.Hash) if err != nil { c.String(http.StatusInternalServerError, err.Error()) diff --git a/server/api/user.go b/server/api/user.go index 857368c3e7..1785f1995a 100644 --- a/server/api/user.go +++ b/server/api/user.go @@ -153,7 +153,9 @@ func GetRepos(c *gin.Context) { // @Param Authorization header string true "Insert your personal access token" default(Bearer ) func PostToken(c *gin.Context) { user := session.User(c) - tokenString, err := token.New(token.UserToken, strconv.FormatInt(user.ID, 10)).Sign(user.Hash) + t := token.New(token.UserToken) + t.Set("user-id", strconv.FormatInt(user.ID, 10)) + tokenString, err := t.Sign(user.Hash) if err != nil { _ = c.AbortWithError(http.StatusInternalServerError, err) return @@ -182,7 +184,9 @@ func DeleteToken(c *gin.Context) { return } - tokenString, err := token.New(token.UserToken, strconv.FormatInt(user.ID, 10)).Sign(user.Hash) + t := token.New(token.UserToken) + t.Set("user-id", strconv.FormatInt(user.ID, 10)) + tokenString, err := t.Sign(user.Hash) if err != nil { _ = c.AbortWithError(http.StatusInternalServerError, err) return diff --git a/server/router/middleware/session/user.go b/server/router/middleware/session/user.go index 1b96b6864e..b413cf5281 100644 --- a/server/router/middleware/session/user.go +++ b/server/router/middleware/session/user.go @@ -45,7 +45,7 @@ func SetUser() gin.HandlerFunc { t, err := token.ParseRequest(c.Request, func(t *token.Token) (string, error) { var err error - userID, err := strconv.ParseInt(t.Text, 10, 64) + userID, err := strconv.ParseInt(t.Get("user-id"), 10, 64) if err != nil { return "", err } diff --git a/server/web/config.go b/server/web/config.go index 987ef128d8..daa3a2fc55 100644 --- a/server/web/config.go +++ b/server/web/config.go @@ -34,10 +34,9 @@ func Config(c *gin.Context) { var csrf string if user != nil { - csrf, _ = token.New( - token.CsrfToken, - strconv.FormatInt(user.ID, 10), - ).Sign(user.Hash) + t := token.New(token.CsrfToken) + t.Set("user-id", strconv.FormatInt(user.ID, 10)) + csrf, _ = t.Sign(user.Hash) } // TODO: remove this and use the forge type from the corresponding repo diff --git a/shared/token/token.go b/shared/token/token.go index d5c48aafd9..83c34a9167 100644 --- a/shared/token/token.go +++ b/shared/token/token.go @@ -37,11 +37,13 @@ const SignerAlgo = "HS256" type Token struct { Kind string - Text string + data map[string]string } func parse(raw string, fn SecretFunc) (*Token, error) { - token := &Token{} + token := &Token{ + data: map[string]string{}, + } parsed, err := jwt.Parse(raw, keyFunc(token, fn)) if err != nil { return nil, err @@ -99,8 +101,8 @@ func CheckCsrf(r *http.Request, fn SecretFunc) error { return err } -func New(kind, text string) *Token { - return &Token{Kind: kind, Text: text} +func New(kind string) *Token { + return &Token{Kind: kind, data: map[string]string{}} } // Sign signs the token using the given secret hash @@ -118,13 +120,25 @@ func (t *Token) SignExpires(secret string, exp int64) (string, error) { return "", fmt.Errorf("token claim is not a MapClaims") } claims["type"] = t.Kind - claims["text"] = t.Text if exp > 0 { claims["exp"] = float64(exp) } + + for k, v := range t.data { + claims[k] = v + } + return token.SignedString([]byte(secret)) } +func (t *Token) Set(key, value string) { + t.data[key] = value +} + +func (t *Token) Get(key string) string { + return t.data[key] +} + func keyFunc(token *Token, fn SecretFunc) jwt.Keyfunc { return func(t *jwt.Token) (any, error) { claims, ok := t.Claims.(jwt.MapClaims) @@ -145,13 +159,16 @@ func keyFunc(token *Token, fn SecretFunc) jwt.Keyfunc { } token.Kind, _ = kind.(string) - // extract the token value and cast to - // expected type. - text, ok := claims["text"] - if !ok { - return nil, jwt.ErrInvalidType + // extract the data claims + for k, v := range claims { + if k == "type" || k == "exp" { + continue + } + + if str, ok := v.(string); ok { + token.data[k] = str + } } - token.Text, _ = text.(string) // invoke the callback function to retrieve // the secret key used to verify From c49bfbe6512e035a130f235177c5b1318639bea2 Mon Sep 17 00:00:00 2001 From: Anbraten <6918444+anbraten@users.noreply.github.com> Date: Tue, 14 May 2024 12:33:50 +0200 Subject: [PATCH 6/8] skip internal claims --- shared/token/token.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared/token/token.go b/shared/token/token.go index 83c34a9167..b2b91693cf 100644 --- a/shared/token/token.go +++ b/shared/token/token.go @@ -161,7 +161,7 @@ func keyFunc(token *Token, fn SecretFunc) jwt.Keyfunc { // extract the data claims for k, v := range claims { - if k == "type" || k == "exp" { + if k == "type" || k == "exp" || k == "nbf" || k == "iat" || k == "aud" || k == "iss" || k == "sub" || k == "jti" { continue } From a4320f1efd47ed61267edf0f70ffdaef2585d8d1 Mon Sep 17 00:00:00 2001 From: Anbraten <6918444+anbraten@users.noreply.github.com> Date: Tue, 14 May 2024 13:02:23 +0200 Subject: [PATCH 7/8] add todo --- docs/docs/91-migrations.md | 7 +++++++ server/api/hook.go | 2 ++ 2 files changed, 9 insertions(+) diff --git a/docs/docs/91-migrations.md b/docs/docs/91-migrations.md index 6ad8f7b13f..c420e08719 100644 --- a/docs/docs/91-migrations.md +++ b/docs/docs/91-migrations.md @@ -2,6 +2,13 @@ Some versions need some changes to the server configuration or the pipeline configuration files. + + ## `next` - Deprecated `steps.[name].group` in favor of `steps.[name].depends_on` (see [workflow syntax](./20-usage/20-workflow-syntax.md#depends_on) to learn how to set dependencies) diff --git a/server/api/hook.go b/server/api/hook.go index 207cf714bf..f69f2d2585 100644 --- a/server/api/hook.go +++ b/server/api/hook.go @@ -189,6 +189,8 @@ func PostHook(c *gin.Context) { c.String(http.StatusBadRequest, msg) return } + + // TODO: remove fallback for text full name in next major release verifiedKey := parsedToken.Get("repo-id") == strconv.FormatInt(repo.ID, 10) || parsedToken.Get("text") == currentRepoFullName if !verifiedKey { verifiedKey, err = _store.HasRedirectionForRepo(repo.ID, repo.FullName) From 85fb52c0af7d05ed41c3b0a3acda62d3fb216bc9 Mon Sep 17 00:00:00 2001 From: Anbraten <6918444+anbraten@users.noreply.github.com> Date: Mon, 20 May 2024 15:12:34 +0200 Subject: [PATCH 8/8] adjust claim copy and add source for the registered claims --- shared/token/token.go | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/shared/token/token.go b/shared/token/token.go index b2b91693cf..9b6504a002 100644 --- a/shared/token/token.go +++ b/shared/token/token.go @@ -36,13 +36,13 @@ const ( const SignerAlgo = "HS256" type Token struct { - Kind string - data map[string]string + Kind string + claims jwt.MapClaims } func parse(raw string, fn SecretFunc) (*Token, error) { token := &Token{ - data: map[string]string{}, + claims: jwt.MapClaims{}, } parsed, err := jwt.Parse(raw, keyFunc(token, fn)) if err != nil { @@ -102,7 +102,7 @@ func CheckCsrf(r *http.Request, fn SecretFunc) error { } func New(kind string) *Token { - return &Token{Kind: kind, data: map[string]string{}} + return &Token{Kind: kind, claims: jwt.MapClaims{}} } // Sign signs the token using the given secret hash @@ -119,24 +119,30 @@ func (t *Token) SignExpires(secret string, exp int64) (string, error) { if !ok { return "", fmt.Errorf("token claim is not a MapClaims") } + + for k, v := range t.claims { + claims[k] = v + } + claims["type"] = t.Kind if exp > 0 { claims["exp"] = float64(exp) } - for k, v := range t.data { - claims[k] = v - } - return token.SignedString([]byte(secret)) } func (t *Token) Set(key, value string) { - t.data[key] = value + t.claims[key] = value } func (t *Token) Get(key string) string { - return t.data[key] + claim, ok := t.claims[key].(string) + if !ok { + return "" + } + + return claim } func keyFunc(token *Token, fn SecretFunc) jwt.Keyfunc { @@ -151,23 +157,25 @@ func keyFunc(token *Token, fn SecretFunc) jwt.Keyfunc { return nil, jwt.ErrSignatureInvalid } - // extract the token kind and cast to - // the expected type. + // extract the token kind and cast to the expected type kind, ok := claims["type"] if !ok { return nil, jwt.ErrInvalidType } token.Kind, _ = kind.(string) - // extract the data claims + // copy custom claims for k, v := range claims { - if k == "type" || k == "exp" || k == "nbf" || k == "iat" || k == "aud" || k == "iss" || k == "sub" || k == "jti" { + // skip the reserved claims https://datatracker.ietf.org/doc/html/rfc7519#section-4.1 + if k == "iss" || k == "sub" || k == "aud" || k == "exp" || k == "nbf" || k == "iat" || k == "jti" { continue } - if str, ok := v.(string); ok { - token.data[k] = str + if k == "type" { + continue } + + token.claims[k] = v } // invoke the callback function to retrieve