mirrors/woodpecker
1
0
Fork 0
mirror of https://github.com/woodpecker-ci/woodpecker.git synced 2025-02-16 11:25:14 +00:00
Code Issues Projects Releases Wiki Activity

Enhance authentication UX (#3807)

Browse source
This commit is contained in:
Anbraten 2024-06-21 09:55:30 +02:00 committed by GitHub
parent fbb96ff8f5
commit 1a39d57f71
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
18 changed files with 116 additions and 187 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

130
server/api/login.go
View file

@ -17,7 +17,9 @@ package api
import (
"encoding/base32"
"errors"
"fmt"
"net/http"
"net/url"
"strconv"
"time"
@ -34,15 +36,24 @@ import (
"go.woodpecker-ci.org/woodpecker/v2/shared/token"
)
func HandleLogin(c *gin.Context) {
if err := c.Request.FormValue("error"); err != "" {
c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login/error?code="+err)
} else {
c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/authorize")
}
}
func HandleAuth(c *gin.Context) {
// TODO: check if this is really needed
c.Writer.Header().Del("Content-Type")
// redirect when getting oauth error from forge to login page
if err := c.Request.FormValue("error"); err != "" {
query := url.Values{}
query.Set("error", err)
if errorDescription := c.Request.FormValue("error_description"); errorDescription != "" {
query.Set("error_description", errorDescription)
}
if errorURI := c.Request.FormValue("error_uri"); errorURI != "" {
query.Set("error_uri", errorURI)
}
c.Redirect(http.StatusSeeOther, fmt.Sprintf("%s/login?%s", server.Config.Server.RootPath, query.Encode()))
return
}
_store := store.FromContext(c)
_forge, err := server.Config.Services.Manager.ForgeMain()
if err != nil {
@ -51,15 +62,9 @@ func HandleAuth(c *gin.Context) {
}
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")
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"),
userFromForge, redirectURL, err := _forge.Login(c, &forge_types.OAuthRequest{
Code: c.Request.FormValue("code"),
State: "woodpecker", // TODO: use proper state
})
if err != nil {
log.Error().Err(err).Msg("cannot authenticate user")
@ -67,13 +72,13 @@ func HandleAuth(c *gin.Context) {
return
}
// The user is not authorized yet -> redirect
if tmpUser == nil {
if userFromForge == nil {
http.Redirect(c.Writer, c.Request, redirectURL, http.StatusSeeOther)
return
}
// get the user from the database
u, err := _store.GetUserRemoteID(tmpUser.ForgeRemoteID, tmpUser.Login)
user, err := _store.GetUserRemoteID(userFromForge.ForgeRemoteID, userFromForge.Login)
if err != nil && !errors.Is(err, types.RecordNotExist) {
_ = c.AbortWithError(http.StatusInternalServerError, err)
return
@ -81,31 +86,31 @@ func HandleAuth(c *gin.Context) {
if errors.Is(err, types.RecordNotExist) {
// if self-registration is disabled we should return a not authorized error
if !server.Config.Permissions.Open && !server.Config.Permissions.Admins.IsAdmin(tmpUser) {
log.Error().Msgf("cannot register %s. registration closed", tmpUser.Login)
c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=access_denied")
if !server.Config.Permissions.Open && !server.Config.Permissions.Admins.IsAdmin(userFromForge) {
log.Error().Msgf("cannot register %s. registration closed", userFromForge.Login)
c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=registration_closed")
return
}
// if self-registration is enabled for allowed organizations we need to
// check the user's organization membership.
if server.Config.Permissions.Orgs.IsConfigured {
teams, terr := _forge.Teams(c, tmpUser)
teams, terr := _forge.Teams(c, userFromForge)
if terr != nil || !server.Config.Permissions.Orgs.IsMember(teams) {
log.Error().Err(terr).Msgf("cannot verify team membership for %s.", u.Login)
log.Error().Err(terr).Msgf("cannot verify team membership for %s.", user.Login)
c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=access_denied")
return
}
}
// create the user account
u = &model.User{
Login: tmpUser.Login,
ForgeRemoteID: tmpUser.ForgeRemoteID,
Token: tmpUser.Token,
Secret: tmpUser.Secret,
Email: tmpUser.Email,
Avatar: tmpUser.Avatar,
user = &model.User{
Login: userFromForge.Login,
ForgeRemoteID: userFromForge.ForgeRemoteID,
Token: userFromForge.Token,
Secret: userFromForge.Secret,
Email: userFromForge.Email,
Avatar: userFromForge.Avatar,
ForgeID: forgeID,
Hash: base32.StdEncoding.EncodeToString(
securecookie.GenerateRandomKey(32),
@ -113,17 +118,17 @@ func HandleAuth(c *gin.Context) {
}
// insert the user into the database
if err := _store.CreateUser(u); err != nil {
log.Error().Err(err).Msgf("cannot insert %s", u.Login)
if err := _store.CreateUser(user); err != nil {
log.Error().Err(err).Msgf("cannot insert %s", user.Login)
c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=internal_error")
return
}
// if another user already have activated repos on behave of that user,
// the user was stored as org. now we adopt it to the user.
if org, err := _store.OrgFindByName(u.Login); err == nil && org != nil {
if org, err := _store.OrgFindByName(user.Login); err == nil && org != nil {
org.IsUser = true
u.OrgID = org.ID
user.OrgID = org.ID
if err := _store.OrgUpdate(org); err != nil {
log.Error().Err(err).Msgf("on user creation, could not mark org as user")
}
@ -133,76 +138,76 @@ func HandleAuth(c *gin.Context) {
return
}
org = &model.Org{
Name: u.Login,
Name: user.Login,
IsUser: true,
Private: false,
ForgeID: u.ForgeID,
ForgeID: user.ForgeID,
}
if err := _store.OrgCreate(org); err != nil {
log.Error().Err(err).Msgf("on user creation, could not create org for user")
}
u.OrgID = org.ID
user.OrgID = org.ID
}
}
// update org name
if u.Login != tmpUser.Login {
org, err := _store.OrgGet(u.OrgID)
if user.Login != userFromForge.Login {
org, err := _store.OrgGet(user.OrgID)
if err != nil {
log.Error().Err(err).Msgf("cannot get org %s", u.Login)
log.Error().Err(err).Msgf("cannot get org %s", user.Login)
c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=internal_error")
return
}
org.Name = u.Login
org.Name = user.Login
if err := _store.OrgUpdate(org); err != nil {
log.Error().Err(err).Msgf("on user creation, could not mark org as user")
}
}
// update the user meta data and authorization data.
u.Token = tmpUser.Token
u.Secret = tmpUser.Secret
u.Email = tmpUser.Email
u.Avatar = tmpUser.Avatar
u.ForgeRemoteID = tmpUser.ForgeRemoteID
u.Login = tmpUser.Login
u.Admin = u.Admin || server.Config.Permissions.Admins.IsAdmin(tmpUser)
user.Token = userFromForge.Token
user.Secret = userFromForge.Secret
user.Email = userFromForge.Email
user.Avatar = userFromForge.Avatar
user.ForgeRemoteID = userFromForge.ForgeRemoteID
user.Login = userFromForge.Login
user.Admin = user.Admin || server.Config.Permissions.Admins.IsAdmin(userFromForge)
// if self-registration is enabled for allowed organizations we need to
// check the user's organization membership.
if server.Config.Permissions.Orgs.IsConfigured {
teams, terr := _forge.Teams(c, u)
teams, terr := _forge.Teams(c, user)
if terr != nil || !server.Config.Permissions.Orgs.IsMember(teams) {
log.Error().Err(terr).Msgf("cannot verify team membership for %s", u.Login)
log.Error().Err(terr).Msgf("cannot verify team membership for %s", user.Login)
c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=access_denied")
return
}
}
if err := _store.UpdateUser(u); err != nil {
log.Error().Err(err).Msgf("cannot update %s", u.Login)
if err := _store.UpdateUser(user); err != nil {
log.Error().Err(err).Msgf("cannot update %s", user.Login)
c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=internal_error")
return
}
exp := time.Now().Add(server.Config.Server.SessionExpires).Unix()
_token := token.New(token.SessToken)
_token.Set("user-id", strconv.FormatInt(u.ID, 10))
tokenString, err := _token.SignExpires(u.Hash, exp)
_token.Set("user-id", strconv.FormatInt(user.ID, 10))
tokenString, err := _token.SignExpires(user.Hash, exp)
if err != nil {
log.Error().Msgf("cannot create token for %s", u.Login)
log.Error().Msgf("cannot create token for %s", user.Login)
c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=internal_error")
return
}
repos, _ := _forge.Repos(c, u)
repos, _ := _forge.Repos(c, user)
for _, forgeRepo := range repos {
dbRepo, err := _store.GetRepoForgeID(forgeRepo.ForgeRemoteID)
if err != nil && errors.Is(err, types.RecordNotExist) {
continue
}
if err != nil {
log.Error().Err(err).Msgf("cannot list repos for %s", u.Login)
log.Error().Err(err).Msgf("cannot list repos for %s", user.Login)
c.Redirect(http.StatusSeeOther, "/login?error=internal_error")
return
}
@ -211,14 +216,14 @@ func HandleAuth(c *gin.Context) {
continue
}
log.Debug().Msgf("synced user permission for %s %s", u.Login, dbRepo.FullName)
log.Debug().Msgf("synced user permission for %s %s", user.Login, dbRepo.FullName)
perm := forgeRepo.Perm
perm.Repo = dbRepo
perm.RepoID = dbRepo.ID
perm.UserID = u.ID
perm.UserID = user.ID
perm.Synced = time.Now().Unix()
if err := _store.PermUpsert(perm); err != nil {
log.Error().Err(err).Msgf("cannot update permissions for %s", u.Login)
log.Error().Err(err).Msgf("cannot update permissions for %s", user.Login)
c.Redirect(http.StatusSeeOther, "/login?error=internal_error")
return
}
@ -235,7 +240,8 @@ func GetLogout(c *gin.Context) {
c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/")
}
func GetLoginToken(c *gin.Context) {
// TODO: remove in 3.0
func DeprecatedGetLoginToken(c *gin.Context) {
_store := store.FromContext(c)
_forge, err := server.Config.Services.Manager.ForgeMain() // TODO: get selected forge from auth request

11
server/forge/bitbucket/bitbucket.go
View file

@ -81,16 +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")
// get the OAuth errors
if req.Error != "" {
return nil, redirectURL, &forge_types.AuthError{
Err: req.Error,
Description: req.ErrorDescription,
URI: req.ErrorURI,
}
}
redirectURL := config.AuthCodeURL(req.State)
// check the OAuth code
if len(req.Code) == 0 {

6
server/forge/bitbucket/bitbucket_test.go
View file

@ -90,12 +90,6 @@ func Test_bitbucket(t *testing.T) {
})
g.Assert(err).IsNotNil()
})
g.It("Should handle authentication errors", func() {
_, _, err := c.Login(ctx, &types.OAuthRequest{
Error: "invalid_scope",
})
g.Assert(err).IsNotNil()
})
})
g.Describe("Given an access token", func() {

12
server/forge/bitbucketdatacenter/bitbucketdatacenter.go
View file

@ -96,16 +96,8 @@ func (c *client) URL() string {
func (c *client) Login(ctx context.Context, req *forge_types.OAuthRequest) (*model.User, string, error) {
config := c.newOAuth2Config()
// TODO: Add proper state and pkce (https://oauth.net/2/pkce/) ...
redirectURL := config.AuthCodeURL("woodpecker")
if req.Error != "" {
return nil, redirectURL, &forge_types.AuthError{
Err: req.Error,
Description: req.ErrorDescription,
URI: req.ErrorURI,
}
}
// TODO: Use pkce flow (https://oauth.net/2/pkce/) ...
redirectURL := config.AuthCodeURL(req.State)
if len(req.Code) == 0 {
return nil, redirectURL, nil

11
server/forge/forgejo/forgejo.go
View file

@ -111,16 +111,7 @@ func (c *Forgejo) oauth2Config(ctx context.Context) (*oauth2.Config, context.Con
// Forgejo account details are returned when the user is successfully authenticated.
func (c *Forgejo) Login(ctx context.Context, req *forge_types.OAuthRequest) (*model.User, string, error) {
config, oauth2Ctx := c.oauth2Config(ctx)
redirectURL := config.AuthCodeURL("woodpecker")
// check the OAuth errors
if req.Error != "" {
return nil, redirectURL, &forge_types.AuthError{
Err: req.Error,
Description: req.ErrorDescription,
URI: req.ErrorURI,
}
}
redirectURL := config.AuthCodeURL(req.State)
// check the OAuth code
if len(req.Code) == 0 {

11
server/forge/gitea/gitea.go
View file

@ -113,16 +113,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")
// check the OAuth errors
if req.Error != "" {
return nil, redirectURL, &forge_types.AuthError{
Err: req.Error,
Description: req.ErrorDescription,
URI: req.ErrorURI,
}
}
redirectURL := config.AuthCodeURL(req.State)
// check the OAuth code
if len(req.Code) == 0 {

14
server/forge/github/github.go
View file

@ -100,16 +100,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")
// check the OAuth errors
if req.Error != "" {
return nil, redirectURL, &forge_types.AuthError{
Err: req.Error,
Description: req.ErrorDescription,
URI: req.ErrorURI,
}
}
redirectURL := config.AuthCodeURL(req.State)
// check the OAuth code
if len(req.Code) == 0 {
@ -662,9 +653,6 @@ func (c *client) getTagCommitSHA(ctx context.Context, repo *model.Repo, tagName
}
gh := c.newClientToken(ctx, user.Token)
if err != nil {
return "", err
}
page := 1
var tag *github.RepositoryTag

11
server/forge/gitlab/gitlab.go
View file

@ -115,16 +115,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")
// check the OAuth errors
if req.Error != "" {
return nil, redirectURL, &forge_types.AuthError{
Err: req.Error,
Description: req.ErrorDescription,
URI: req.ErrorURI,
}
}
redirectURL := config.AuthCodeURL(req.State)
// check the OAuth code
if len(req.Code) == 0 {

22
server/forge/types/errors.go
View file

@ -21,28 +21,6 @@ import (
"strings"
)
// AuthError represents forge authentication error.
type AuthError struct {
Err string
Description string
URI string
}
// Error implements error interface.
func (ae *AuthError) Error() string {
err := ae.Err
if ae.Description != "" {
err += " " + ae.Description
}
if ae.URI != "" {
err += " " + ae.URI
}
return err
}
// Check interface implementation at compile time.
var _ error = new(AuthError)
var ErrNotImplemented = errors.New("not implemented")
type ErrIgnoreEvent struct {

6
server/forge/types/oauth.go
View file

@ -15,8 +15,6 @@
package types
type OAuthRequest struct {
Error string
ErrorURI string
ErrorDescription string
Code string
Code string
State string
}

3
server/router/router.go
View file

@ -58,12 +58,11 @@ func Load(noRouteHandler http.HandlerFunc, middleware ...gin.HandlerFunc) http.H
base.GET("/web-config.js", web.Config)
base.GET("/logout", api.GetLogout)
base.GET("/login", api.HandleLogin)
auth := base.Group("/authorize")
{
auth.GET("", api.HandleAuth)
auth.POST("", api.HandleAuth)
auth.POST("/token", api.GetLoginToken)
auth.POST("/token", api.DeprecatedGetLoginToken)
}
base.GET("/metrics", metrics.PromHandler())

11
web/src/assets/locales/en.json
View file

@ -409,10 +409,7 @@
"reset_token": "Reset token",
"swagger_ui": "Swagger UI"
}
},
"oauth_error": "Error while authenticating against OAuth provider",
"internal_error": "Some internal error occurred",
"access_denied": "You are not allowed to login"
}
},
"secrets": {
"secrets": "Secrets",
@ -451,5 +448,9 @@
"cli_login_failed": "Login to CLI failed",
"cli_login_denied": "Login to CLI denied",
"return_to_cli": "You can now close this tab and return to the CLI.",
"settings": "Settings"
"settings": "Settings",
"oauth_error": "Error while authenticating against OAuth provider",
"internal_error": "Some internal error occurred",
"registration_closed": "The registration is closed",
"access_denied": "You are not allowed to access this instance"
}

1
web/src/components/admin/settings/AdminInfoTab.vue
View file

@ -29,6 +29,7 @@
<script lang="ts" setup>
import WoodpeckerLogo from '~/assets/logo.svg?component';
import Error from '~/components/atomic/Error.vue';
import Settings from '~/components/layout/Settings.vue';
import { useVersion } from '~/compositions/useVersion';

4
web/src/components/atomic/Error.vue
View file

@ -4,7 +4,7 @@
>
<Icon v-if="!textOnly" name="error" />
<slot>
<span>{{ text }}</span>
<span class="whitespace-pre">{{ text }}</span>
</slot>
</div>
</template>
@ -12,6 +12,6 @@
<script lang="ts" setup>
defineProps<{
textOnly?: boolean;
text: string;
text?: string;
}>();
</script>

4
web/src/components/atomic/Warning.vue
View file

@ -4,7 +4,7 @@
>
<Icon v-if="!textOnly" name="warning" />
<slot>
<span>{{ text }}</span>
<span class="whitespace-pre">{{ text }}</span>
</slot>
</div>
</template>
@ -12,6 +12,6 @@
<script lang="ts" setup>
defineProps<{
textOnly?: boolean;
text: string;
text?: string;
}>();
</script>

2
web/src/compositions/useAuthentication.ts
View file

@ -12,6 +12,6 @@ export default () =>
const config = useUserConfig();
config.setUserConfig('redirectUrl', url);
}
window.location.href = `${useConfig().rootPath}/login`;
window.location.href = `${useConfig().rootPath}/authorize`;
},
}) as const;

11
web/src/router.ts
View file

@ -153,14 +153,7 @@ const routes: RouteRecordRaw[] = [
props: true,
},
{
path: `${rootPath}/login/error`,
name: 'login-error',
component: (): Component => import('~/views/Login.vue'),
meta: { blank: true },
props: true,
},
{
path: `${rootPath}/do-login`,
path: `${rootPath}/login`,
name: 'login',
component: (): Component => import('~/views/Login.vue'),
meta: { blank: true },
@ -199,7 +192,7 @@ const router = createRouter({
router.beforeEach(async (to, _, next) => {
const config = useUserConfig();
const { redirectUrl } = config.userConfig.value;
if (redirectUrl !== '') {
if (redirectUrl !== '' && to.name !== 'login') {
config.setUserConfig('redirectUrl', '');
next(redirectUrl);
}

33
web/src/views/Login.vue
View file

@ -1,7 +1,17 @@
<template>
<main class="flex flex-col w-full h-full justify-center items-center">
<!-- TODO: Should use vue notifications. -->
<Error v-if="errorMessage" text-only :text="errorMessage" class="w-full md:w-3xl" />
<Error v-if="errorMessage" class="w-full md:w-3xl">
<span class="whitespace-pre">{{ errorMessage }}</span>
<span v-if="errorDescription" class="whitespace-pre mt-1">{{ errorDescription }}</span>
<a
v-if="errorUri"
:href="errorUri"
target="_blank"
class="text-wp-link-100 hover:text-wp-link-200 cursor-pointer mt-1"
>
<span>{{ errorUri }}</span>
</a>
</Error>
<div
class="flex flex-col w-full overflow-hidden bg-wp-background-100 shadow border border-wp-background-400 dark:bg-wp-background-200 md:m-8 md:rounded-md md:flex-row md:w-3xl md:h-sm"
@ -24,12 +34,12 @@ import { useRoute, useRouter } from 'vue-router';
import WoodpeckerLogo from '~/assets/logo.svg?component';
import Button from '~/components/atomic/Button.vue';
import Error from '~/components/atomic/Error.vue';
import useAuthentication from '~/compositions/useAuthentication';
const route = useRoute();
const router = useRouter();
const authentication = useAuthentication();
const errorMessage = ref<string>();
const i18n = useI18n();
function doLogin() {
@ -38,20 +48,25 @@ function doLogin() {
}
const authErrorMessages = {
oauth_error: i18n.t('user.oauth_error'),
internal_error: i18n.t('user.internal_error'),
access_denied: i18n.t('user.access_denied'),
oauth_error: i18n.t('oauth_error'),
internal_error: i18n.t('internal_error'),
registration_closed: i18n.t('registration_closed'),
access_denied: i18n.t('access_denied'),
};
const errorMessage = ref<string>();
const errorDescription = ref<string>(route.query.error_description as string);
const errorUri = ref<string>(route.query.error_uri as string);
onMounted(async () => {
if (authentication.isAuthenticated) {
await router.replace({ name: 'home' });
return;
}
if (route.query.code) {
const code = route.query.code as keyof typeof authErrorMessages;
errorMessage.value = authErrorMessages[code];
if (route.query.error) {
const error = route.query.error as keyof typeof authErrorMessages;
errorMessage.value = authErrorMessages[error] ?? error;
}
});
</script>