diff --git a/config.yml.sample b/config.yml.sample index 88465a03d..19a3feb11 100644 --- a/config.yml.sample +++ b/config.yml.sample @@ -6,7 +6,7 @@ service: # The duration of the issued JWT tokens in seconds. # The default is 259200 seconds (3 Days). jwtttl: 259200 - # The duration of the "remember me" time in seconds. When the login request is made with + # The duration of the "remember me" time in seconds. When the login request is made with # the long param set, the token returned will be valid for this period. # The default is 2592000 seconds (30 Days). jwtttllong: 2592000 @@ -48,7 +48,7 @@ service: # If enabled, vikunja will send an email to everyone who is either assigned to a task or created it when a task reminder # is due. enableemailreminders: true - # If true, will allow users to request the complete deletion of their account. When using external authentication methods + # If true, will allow users to request the complete deletion of their account. When using external authentication methods # it may be required to coordinate with them in order to delete the account. This setting will not affect the cli commands # for user deletion. enableuserdeletion: true @@ -109,7 +109,7 @@ database: typesense: # Whether to enable the Typesense integration. If true, all tasks will be synced to the configured Typesense # instance and all search and filtering will run through Typesense instead of only through the database. - # Typesense allows fast fulltext search including fuzzy matching support. It may return different results than + # Typesense allows fast fulltext search including fuzzy matching support. It may return different results than # what you'd get with a database-only search. enabled: false # The url to the Typesense instance you want to use. Can be hosted locally or in Typesense Cloud as long @@ -203,7 +203,7 @@ ratelimit: # Possible values are "keyvalue", "memory" or "redis". # When choosing "keyvalue" this setting follows the one configured in the "keyvalue" section. store: keyvalue - # The number of requests a user can make from the same IP to all unauthenticated routes (login, register, + # The number of requests a user can make from the same IP to all unauthenticated routes (login, register, # password confirmation, email verification, password reset request) per minute. This limit cannot be disabled. # You should only change this if you know what you're doing. noauthlimit: 10 @@ -325,6 +325,10 @@ auth: clientid: # The client secret used to authenticate Vikunja at the OpenID Connect provider. clientsecret: + # The scope necessary to use oidc. + # If you want to use the Feature to create and assign to vikunja teams via oidc, you have to add the custom "vikunja_scope" and check [openid.md](https://vikunja.io/docs/openid/). + # e.g. scope: openid email profile vikunja_scope + scope: openid email profile # Prometheus metrics endpoint metrics: diff --git a/docs/content/doc/setup/config.md b/docs/content/doc/setup/config.md index a24236524..6b64227c7 100644 --- a/docs/content/doc/setup/config.md +++ b/docs/content/doc/setup/config.md @@ -94,7 +94,7 @@ Environment path: `VIKUNJA_SERVICE_JWTTTL` ### jwtttllong -The duration of the "remember me" time in seconds. When the login request is made with +The duration of the "remember me" time in seconds. When the login request is made with the long param set, the token returned will be valid for this period. The default is 2592000 seconds (30 Days). @@ -289,7 +289,7 @@ Environment path: `VIKUNJA_SERVICE_ENABLEEMAILREMINDERS` ### enableuserdeletion -If true, will allow users to request the complete deletion of their account. When using external authentication methods +If true, will allow users to request the complete deletion of their account. When using external authentication methods it may be required to coordinate with them in order to delete the account. This setting will not affect the cli commands for user deletion. @@ -569,7 +569,7 @@ Environment path: `VIKUNJA_DATABASE_TLS` Whether to enable the Typesense integration. If true, all tasks will be synced to the configured Typesense instance and all search and filtering will run through Typesense instead of only through the database. -Typesense allows fast fulltext search including fuzzy matching support. It may return different results than +Typesense allows fast fulltext search including fuzzy matching support. It may return different results than what you'd get with a database-only search. Default: `false` @@ -1024,7 +1024,7 @@ Environment path: `VIKUNJA_RATELIMIT_STORE` ### noauthlimit -The number of requests a user can make from the same IP to all unauthenticated routes (login, register, +The number of requests a user can make from the same IP to all unauthenticated routes (login, register, password confirmation, email verification, password reset request) per minute. This limit cannot be disabled. You should only change this if you know what you're doing. diff --git a/docs/content/doc/setup/openid-examples.md b/docs/content/doc/setup/openid-examples.md index 3f7d37ca6..dd3165b79 100644 --- a/docs/content/doc/setup/openid-examples.md +++ b/docs/content/doc/setup/openid-examples.md @@ -67,7 +67,7 @@ Google config: Note that there currently seems to be no way to stop creation of new users, even when `enableregistration` is `false` in the configuration. This means that this approach works well only with an "Internal Organization" app for Google Workspace, which limits the allowed users to organizational accounts only. External / public applications will potentially allow every Google user to register. -## Keycloak +## Keycloak Vikunja Config: ```yaml diff --git a/docs/content/doc/setup/openid.md b/docs/content/doc/setup/openid.md new file mode 100644 index 000000000..12121930a --- /dev/null +++ b/docs/content/doc/setup/openid.md @@ -0,0 +1,97 @@ +# OpenID + +Vikunja allows for authentication with an oauth provider via the OpenID standard. + +To learn more about how to configure this, [check out the examples]({{< ref "openid-examples.md">}}) + +{{< table_of_contents >}} + +## Automatically assign users to teams + +Vikunja is capable of automatically adding users to a team based on a group defined in the oidc provider. +If configured, Vikunja will sync teams, automatically create new ones and make sure the members are part of the configured teams. +Teams which exist only because they were created from oidc attributes are not editable in Vikunja. + +To distinguish between teams created in Vikunja and teams generated automatically via oidc, generated teams have an `oidcID` assigned internally. + +You need to make sure the OpenID provider offers a `vikunja_groups` key through your custom scope. This is the key, which is looked up by Vikunja to start the procedure. + +Additionally, make sure to deliver an `oidcID` and a `name` attribute within the `vikunja_groups`. You can see how to set this up, if you continue reading. + +### Setup in Authentik + +To configure automatic team management through Authentik, we assume you have already [set up Authentik]({{< ref "openid-examples.md">}}#authentik) as an oidc provider for authentication with Vikunja. + +To use Authentik's group assignment feature, follow these steps: + +1. Edit [your config]({{< ref "config.md">}}) to include the following scopes: `openid profile email vikunja_scope` +2. Open `/if/admin/#/core/property-mappings` +3. Create a new property mapping called `vikunja_scope` as scope mapping. There is a field `expression` to enter python expressions that will be delivered with the oidc token. +4. Write a small script like the following to add group information to `vikunja_scope`: + +```python +groupsDict = {"vikunja_groups": []} +for group in request.user.ak_groups.all(): + groupsDict["vikunja_groups"].append({"name": group.name, "oidcID": group.num_pk}) +return groupsDict +``` + +output example: + +``` +{ + "vikunja_groups": [ + { + "name": "team 1", + "oidcID": 33349 + }, + { + "name": "team 2", + "oidcID": 35933 + } + ] +} +``` + +5. In Authentik's menu on the left, go to Applications > Providers > Select the Vikunja provider. Then click on "Edit", on the bottom open "Advanced protocol settings", select the newly created property mapping under "Scopes". Save the provider. + +Now when you log into Vikunja via Authentik it will show you a list of scopes you are claiming. +You should see the description you entered on the oidc provider's admin area. + +Proceed to vikunja and open the teams page in the sidebar menu. +You should see "(sso: *your_oidcID*)" written next to each team you were assigned through oidc. + +## Setup in Keycloak + +The kind people from the Darmstadt Makerspace have written [a guide on how to create a mapper for Vikunja here](https://github.com/makerspace-darmstadt/keycloak-vikunja-mapper). + +## Use cases + +All examples assume one team called "Team 1" in your provider. + +* *Token delivers team.name +team.oidcID and Vikunja team does not exist:* \ +New team will be created called "Team 1" with attribute oidcID: "33929" + +2. *In Vikunja Team with name "team 1" already exists in vikunja, but has no oidcID set:* \ +new team will be created called "team 1" with attribute oidcID: "33929" + +3. *In Vikunja Team with name "team 1" already exists in vikunja, but has different oidcID set:* \ +new team will be created called "team 1" with attribute oidcID: "33929" + +4. *In Vikunja Team with oidcID "33929" already exists in vikunja, but has different name than "team1":* \ +new team will be created called "team 1" with attribute oidcID: "33929" + +5. *Scope vikunja_scope is not set:* \ +nothing happens + +6. *oidcID is not set:* \ +You'll get error. +Custom Scope malformed +"The custom scope set by the OIDC provider is malformed. Please make sure the openid provider sets the data correctly for your scope. Check especially to have set an oidcID." + +7. *In Vikunja I am in "team 3" with oidcID "", but the token does not deliver any data for "team 3":* \ +You will stay in team 3 since it was not set by the oidc provider + +8. *In Vikunja I am in "team 3" with oidcID "12345", but the token does not deliver any data for "team 3"*:\ +You will be signed out of all teams, which have an oidcID set and are not contained in the token. +Especially if you've been the last team member, the team will be deleted. diff --git a/docs/content/doc/usage/errors.md b/docs/content/doc/usage/errors.md index f3a6c8e4f..1ff2ea801 100644 --- a/docs/content/doc/usage/errors.md +++ b/docs/content/doc/usage/errors.md @@ -44,6 +44,7 @@ This document describes the different errors Vikunja can return. | 1020 | 412 | This user account is disabled. | | 1021 | 412 | This account is managed by a third-party authentication provider. | | 1021 | 412 | The username must not contain spaces. | +| 1022 | 412 | The custom scope set by the OIDC provider is malformed. Please make sure the openid provider sets the data correctly for your scope. Check especially to have set an oidcID. | ## Validation @@ -106,6 +107,9 @@ This document describes the different errors Vikunja can return. | 6005 | 409 | The user is already a member of that team. | | 6006 | 400 | Cannot delete the last team member. | | 6007 | 403 | The team does not have access to the project to perform that action. | +| 6008 | 400 | There are no teams found with that team name. | +| 6009 | 400 | There is no oidc team with that team name and oidcId. | +| 6010 | 400 | There are no oidc teams found for the user. | ## User Project Access diff --git a/frontend/src/helpers/redirectToProvider.ts b/frontend/src/helpers/redirectToProvider.ts index 236394de8..00f063aa0 100644 --- a/frontend/src/helpers/redirectToProvider.ts +++ b/frontend/src/helpers/redirectToProvider.ts @@ -11,14 +11,17 @@ export function getRedirectUrlFromCurrentFrontendPath(provider: IProvider): stri export const redirectToProvider = (provider: IProvider) => { - console.log({provider}) - const redirectUrl = getRedirectUrlFromCurrentFrontendPath(provider) const state = createRandomID(24) localStorage.setItem('state', state) - window.location.href = `${provider.authUrl}?client_id=${provider.clientId}&redirect_uri=${redirectUrl}&response_type=code&scope=openid email profile&state=${state}` + let scope = 'openid email profile' + if (provider.scope !== null){ + scope = provider.scope + } + window.location.href = `${provider.authUrl}?client_id=${provider.clientId}&redirect_uri=${redirectUrl}&response_type=code&scope=${scope}&state=${state}` } + export const redirectToProviderOnLogout = (provider: IProvider) => { if (provider.logoutUrl.length > 0) { window.location.href = `${provider.logoutUrl}` diff --git a/frontend/src/modelTypes/ITeam.ts b/frontend/src/modelTypes/ITeam.ts index cd5a7dc73..3cdaae987 100644 --- a/frontend/src/modelTypes/ITeam.ts +++ b/frontend/src/modelTypes/ITeam.ts @@ -9,6 +9,7 @@ export interface ITeam extends IAbstract { description: string members: ITeamMember[] right: Right + oidcId: string createdBy: IUser created: Date diff --git a/frontend/src/models/team.ts b/frontend/src/models/team.ts index 9d86ca8c4..1e75738bb 100644 --- a/frontend/src/models/team.ts +++ b/frontend/src/models/team.ts @@ -13,6 +13,7 @@ export default class TeamModel extends AbstractModel implements ITeam { description = '' members: ITeamMember[] = [] right: Right = RIGHTS.READ + oidcId = '' createdBy: IUser = {} // FIXME: seems wrong created: Date = null diff --git a/frontend/src/types/IProvider.ts b/frontend/src/types/IProvider.ts index 420728926..2dec662a6 100644 --- a/frontend/src/types/IProvider.ts +++ b/frontend/src/types/IProvider.ts @@ -4,4 +4,5 @@ export interface IProvider { authUrl: string; clientId: string; logoutUrl: string; + scope: string; } diff --git a/frontend/src/views/teams/EditTeam.vue b/frontend/src/views/teams/EditTeam.vue index 03cc7e96c..5ee820175 100644 --- a/frontend/src/views/teams/EditTeam.vue +++ b/frontend/src/views/teams/EditTeam.vue @@ -4,7 +4,7 @@ :class="{ 'is-loading': teamService.loading }" > @@ -77,7 +77,7 @@ :padding="false" >
diff --git a/frontend/src/views/teams/ListTeams.vue b/frontend/src/views/teams/ListTeams.vue index a7f01c7d0..3f57133fa 100644 --- a/frontend/src/views/teams/ListTeams.vue +++ b/frontend/src/views/teams/ListTeams.vue @@ -17,11 +17,13 @@ class="teams box" >
  • - - {{ team.name }} + +

    + {{ t.name + (t.oidcId ? ` (sso: ${t.oidcId})`: '') }} +

    • diff --git a/magefile.go b/magefile.go index f58040b6c..569257455 100644 --- a/magefile.go +++ b/magefile.go @@ -25,7 +25,6 @@ import ( "context" "crypto/sha256" "fmt" - "github.com/iancoleman/strcase" "io" "os" "os/exec" @@ -34,6 +33,8 @@ import ( "strings" "time" + "github.com/iancoleman/strcase" + "github.com/magefile/mage/mg" "golang.org/x/sync/errgroup" "gopkg.in/yaml.v3" diff --git a/pkg/db/fixtures/team_members.yml b/pkg/db/fixtures/team_members.yml index eb671be41..889322b7b 100644 --- a/pkg/db/fixtures/team_members.yml +++ b/pkg/db/fixtures/team_members.yml @@ -55,3 +55,7 @@ team_id: 13 user_id: 10 created: 2018-12-01 15:13:12 +- + team_id: 14 + user_id: 10 + created: 2018-12-01 15:13:12 \ No newline at end of file diff --git a/pkg/db/fixtures/teams.yml b/pkg/db/fixtures/teams.yml index b7d347df4..defdedd62 100644 --- a/pkg/db/fixtures/teams.yml +++ b/pkg/db/fixtures/teams.yml @@ -28,4 +28,8 @@ created_by_id: 7 - id: 13 name: testteam13 - created_by_id: 7 \ No newline at end of file + created_by_id: 7 +- id: 14 + name: testteam14 + created_by_id: 7 + oidc_id: 14 \ No newline at end of file diff --git a/pkg/migration/20230104152903.go b/pkg/migration/20230104152903.go new file mode 100644 index 000000000..82747d5f4 --- /dev/null +++ b/pkg/migration/20230104152903.go @@ -0,0 +1,43 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public Licensee as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public Licensee for more details. +// +// You should have received a copy of the GNU Affero General Public Licensee +// along with this program. If not, see . + +package migration + +import ( + "src.techknowlogick.com/xormigrate" + "xorm.io/xorm" +) + +type teams20230104152903 struct { + OidcID string `xorm:"varchar(250) null" maxLength:"250" json:"oidc_id"` +} + +func (teams20230104152903) TableName() string { + return "teams" +} + +func init() { + migrations = append(migrations, &xormigrate.Migration{ + ID: "20230104152903", + Description: "Adding OidcID to teams", + Migrate: func(tx *xorm.Engine) error { + return tx.Sync2(teams20230104152903{}) + }, + Rollback: func(tx *xorm.Engine) error { + return nil + }, + }) +} diff --git a/pkg/models/error.go b/pkg/models/error.go index b20267b55..2e31a6e58 100644 --- a/pkg/models/error.go +++ b/pkg/models/error.go @@ -1059,7 +1059,6 @@ func (err ErrTeamNameCannotBeEmpty) HTTPError() web.HTTPError { return web.HTTPError{HTTPCode: http.StatusBadRequest, Code: ErrCodeTeamNameCannotBeEmpty, Message: "The team name cannot be empty"} } -// ErrTeamDoesNotExist represents an error where a team does not exist type ErrTeamDoesNotExist struct { TeamID int64 } @@ -1178,6 +1177,54 @@ func (err ErrTeamDoesNotHaveAccessToProject) HTTPError() web.HTTPError { return web.HTTPError{HTTPCode: http.StatusForbidden, Code: ErrCodeTeamDoesNotHaveAccessToProject, Message: "This team does not have access to the project."} } +// ErrOIDCTeamDoesNotExist represents an error where a team with specified name and specified oidcId property does not exist +type ErrOIDCTeamDoesNotExist struct { + OidcID string + Name string +} + +// IsErrOIDCTeamDoesNotExist checks if an error is ErrOIDCTeamDoesNotExist. +func IsErrOIDCTeamDoesNotExist(err error) bool { + _, ok := err.(ErrOIDCTeamDoesNotExist) + return ok +} + +// ErrTeamDoesNotExist represents an error where a team does not exist +func (err ErrOIDCTeamDoesNotExist) Error() string { + return fmt.Sprintf("No team with that name and valid oidcId could be found. [Team Name: %v] [OidcID : %v] ", err.Name, err.OidcID) +} + +// ErrCodeTeamDoesNotExist holds the unique world-error code of this error +const ErrCodeOIDCTeamDoesNotExist = 6008 + +// HTTPError holds the http error description +func (err ErrOIDCTeamDoesNotExist) HTTPError() web.HTTPError { + return web.HTTPError{HTTPCode: http.StatusNotFound, Code: ErrCodeTeamDoesNotExist, Message: "No team with that name and valid oidcId could be found."} +} + +// ErrOIDCTeamsDoNotExistForUser represents an error where an oidcTeam does not exist for the user +type ErrOIDCTeamsDoNotExistForUser struct { + UserID int64 +} + +// IsErrOIDCTeamsDoNotExistForUser checks if an error is ErrOIDCTeamsDoNotExistForUser. +func IsErrOIDCTeamsDoNotExistForUser(err error) bool { + _, ok := err.(ErrOIDCTeamsDoNotExistForUser) + return ok +} + +func (err ErrOIDCTeamsDoNotExistForUser) Error() string { + return fmt.Sprintf("No teams with property oidcId could be found for user [User ID: %d]", err.UserID) +} + +// ErrCodeTeamDoesNotExist holds the unique world-error code of this error +const ErrCodeOIDCTeamsDoNotExistForUser = 6009 + +// HTTPError holds the http error description +func (err ErrOIDCTeamsDoNotExistForUser) HTTPError() web.HTTPError { + return web.HTTPError{HTTPCode: http.StatusNotFound, Code: ErrCodeTeamDoesNotExist, Message: "No Teams with property oidcId could be found for User."} +} + // ==================== // User <-> Project errors // ==================== diff --git a/pkg/models/team_members.go b/pkg/models/team_members.go index 829661214..a06cf83db 100644 --- a/pkg/models/team_members.go +++ b/pkg/models/team_members.go @@ -44,7 +44,6 @@ func (tm *TeamMember) Create(s *xorm.Session, a web.Auth) (err error) { if err != nil { return err } - // Check if the user exists member, err := user2.GetUserByUsername(s, tm.Username) if err != nil { @@ -109,6 +108,12 @@ func (tm *TeamMember) Delete(s *xorm.Session, _ web.Auth) (err error) { return } +func (tm *TeamMember) MembershipExists(s *xorm.Session) (exists bool, err error) { + return s. + Where("team_id = ? AND user_id = ?", tm.TeamID, tm.UserID). + Exist(&TeamMember{}) +} + // Update toggles a team member's admin status // @Summary Toggle a team member's admin status // @Description If a user is team admin, this will make them member and vise-versa. diff --git a/pkg/models/teams.go b/pkg/models/teams.go index 1b3deea8b..189250a90 100644 --- a/pkg/models/teams.go +++ b/pkg/models/teams.go @@ -38,6 +38,8 @@ type Team struct { // The team's description. Description string `xorm:"longtext null" json:"description"` CreatedByID int64 `xorm:"bigint not null INDEX" json:"-"` + // The team's oidc id delivered by the oidc provider + OidcID string `xorm:"varchar(250) null" maxLength:"250" json:"oidc_id"` // The user who created this team. CreatedBy *user.User `xorm:"-" json:"created_by"` @@ -91,6 +93,13 @@ type TeamUser struct { TeamID int64 `json:"-"` } +// OIDCTeamData is the relevant data for a team and is delivered by oidc token +type OIDCTeamData struct { + TeamName string + OidcID string + Description string +} + // GetTeamByID gets a team by its ID func GetTeamByID(s *xorm.Session, id int64) (team *Team, err error) { if id < 1 { @@ -120,6 +129,34 @@ func GetTeamByID(s *xorm.Session, id int64) (team *Team, err error) { return } +// GetTeamByOidcIDAndName gets teams where oidc_id and name match parameters +// For oidc team creation oidcID and Name need to be set +func GetTeamByOidcIDAndName(s *xorm.Session, oidcID string, teamName string) (*Team, error) { + team := &Team{} + has, err := s. + Table("teams"). + Where("oidc_id = ? AND name = ?", oidcID, teamName). + Get(team) + if !has || err != nil { + return nil, ErrOIDCTeamDoesNotExist{teamName, oidcID} + } + return team, nil +} + +func FindAllOidcTeamIDsForUser(s *xorm.Session, userID int64) (ts []int64, err error) { + err = s. + Table("team_members"). + Where("user_id = ? ", userID). + Join("RIGHT", "teams", "teams.id = team_members.team_id"). + Where("teams.oidc_id != ? AND teams.oidc_id IS NOT NULL", ""). + Cols("teams.id"). + Find(&ts) + if ts == nil || err != nil { + return ts, err + } + return ts, nil +} + func addMoreInfoToTeams(s *xorm.Session, teams []*Team) (err error) { if len(teams) == 0 { @@ -270,7 +307,6 @@ func (t *Team) Create(s *xorm.Session, a web.Auth) (err error) { return } - // Insert the current user as member and admin tm := TeamMember{TeamID: t.ID, Username: doer.Username, Admin: true} if err = tm.Create(s, doer); err != nil { return err diff --git a/pkg/modules/auth/openid/openid.go b/pkg/modules/auth/openid/openid.go index 618dcd531..1f80445cc 100644 --- a/pkg/modules/auth/openid/openid.go +++ b/pkg/modules/auth/openid/openid.go @@ -21,21 +21,22 @@ import ( "encoding/json" "errors" "net/http" + "strconv" "strings" "code.vikunja.io/web/handler" "code.vikunja.io/api/pkg/db" - "xorm.io/xorm" - "code.vikunja.io/api/pkg/log" "code.vikunja.io/api/pkg/models" "code.vikunja.io/api/pkg/modules/auth" "code.vikunja.io/api/pkg/user" + "code.vikunja.io/api/pkg/utils" "github.com/coreos/go-oidc/v3/oidc" petname "github.com/dustinkirkland/golang-petname" "github.com/labstack/echo/v4" "golang.org/x/oauth2" + "xorm.io/xorm" ) // Callback contains the callback after an auth request was made and redirected @@ -53,16 +54,17 @@ type Provider struct { AuthURL string `json:"auth_url"` LogoutURL string `json:"logout_url"` ClientID string `json:"client_id"` + Scope string `json:"scope"` ClientSecret string `json:"-"` openIDProvider *oidc.Provider Oauth2Config *oauth2.Config `json:"-"` } - type claims struct { - Email string `json:"email"` - Name string `json:"name"` - PreferredUsername string `json:"preferred_username"` - Nickname string `json:"nickname"` + Email string `json:"email"` + Name string `json:"name"` + PreferredUsername string `json:"preferred_username"` + Nickname string `json:"nickname"` + VikunjaGroups []map[string]interface{} `json:"vikunja_groups"` } func init() { @@ -96,6 +98,7 @@ func HandleCallback(c echo.Context) error { // Check if the provider exists providerKey := c.Param("provider") provider, err := GetProvider(providerKey) + log.Debugf("Provider: %v", provider) if err != nil { log.Error(err) return handler.HandleHTTPError(err, c) @@ -145,6 +148,7 @@ func HandleCallback(c echo.Context) error { // Extract custom claims cl := &claims{} + err = idToken.Claims(cl) if err != nil { log.Errorf("Error getting token claims for provider %s: %v", provider.Name, err) @@ -198,16 +202,166 @@ func HandleCallback(c echo.Context) error { return handler.HandleHTTPError(err, c) } + // does the oidc token contain well formed "vikunja_groups" through vikunja_scope + log.Debugf("Checking for vikunja_groups in token %v", cl.VikunjaGroups) + teamData, errs := getTeamDataFromToken(cl.VikunjaGroups, provider) + if len(teamData) > 0 { + for _, err := range errs { + log.Errorf("Error creating teams for user and vikunja groups %s: %v", cl.VikunjaGroups, err) + } + + //find old teams for user through oidc + oldOidcTeams, err := models.FindAllOidcTeamIDsForUser(s, u.ID) + if err != nil { + log.Debugf("No oidc teams found for user %v", err) + } + oidcTeams, err := AssignOrCreateUserToTeams(s, u, teamData) + if err != nil { + log.Errorf("Could not proceed with group routine %v", err) + } + teamIDsToLeave := utils.NotIn(oldOidcTeams, oidcTeams) + err = RemoveUserFromTeamsByIds(s, u, teamIDsToLeave) + if err != nil { + log.Errorf("Found error while leaving teams %v", err) + } + errors := RemoveEmptySSOTeams(s, teamIDsToLeave) + if len(errors) > 0 { + for _, err := range errors { + log.Errorf("Found error while removing empty teams %v", err) + } + } + } err = s.Commit() if err != nil { + _ = s.Rollback() + log.Errorf("Error creating new team for provider %s: %v", provider.Name, err) return handler.HandleHTTPError(err, c) } - // Create token return auth.NewUserAuthTokenResponse(u, c, false) } +func AssignOrCreateUserToTeams(s *xorm.Session, u *user.User, teamData []models.OIDCTeamData) (oidcTeams []int64, err error) { + if len(teamData) == 0 { + return + } + // check if we have seen these teams before. + // find or create Teams and assign user as teammember. + teams, err := GetOrCreateTeamsByOIDCAndNames(s, teamData, u) + if err != nil { + log.Errorf("Error verifying team for %v, got %v. Error: %v", u.Name, teams, err) + return nil, err + } + for _, team := range teams { + tm := models.TeamMember{TeamID: team.ID, UserID: u.ID, Username: u.Username} + exists, _ := tm.MembershipExists(s) + if !exists { + err = tm.Create(s, u) + if err != nil { + log.Errorf("Could not assign user %s to team %s: %v", u.Username, team.Name, err) + } + } + oidcTeams = append(oidcTeams, team.ID) + } + return oidcTeams, err +} + +func RemoveEmptySSOTeams(s *xorm.Session, teamIDs []int64) (errs []error) { + for _, teamID := range teamIDs { + count, err := s.Where("team_id = ?", teamID).Count(&models.TeamMember{}) + if count == 0 && err == nil { + log.Debugf("SSO team with id %v has no members. It will be deleted", teamID) + _, _err := s.Where("id = ?", teamID).Delete(&models.Team{}) + if _err != nil { + errs = append(errs, _err) + } + } + } + return errs +} + +func RemoveUserFromTeamsByIds(s *xorm.Session, u *user.User, teamIDs []int64) (err error) { + + if len(teamIDs) < 1 { + return nil + } + + log.Debugf("Removing team_member with user_id %v from team_ids %v", u.ID, teamIDs) + _, err = s.In("team_id", teamIDs).And("user_id = ?", u.ID).Delete(&models.TeamMember{}) + return err +} + +func getTeamDataFromToken(groups []map[string]interface{}, provider *Provider) (teamData []models.OIDCTeamData, errs []error) { + teamData = []models.OIDCTeamData{} + errs = []error{} + for _, team := range groups { + var name string + var description string + var oidcID string + _, exists := team["name"] + if exists { + name = team["name"].(string) + } + _, exists = team["description"] + if exists { + description = team["description"].(string) + } + _, exists = team["oidcID"] + if exists { + switch t := team["oidcID"].(type) { + case int64: + oidcID = strconv.FormatInt(team["oidcID"].(int64), 10) + case string: + oidcID = string(team["oidcID"].(string)) + case float64: + oidcID = strconv.FormatFloat(team["oidcID"].(float64), 'f', -1, 64) + default: + log.Errorf("No oidcID assigned for %v or type %v not supported", team, t) + } + } + if name == "" || oidcID == "" { + log.Errorf("Claim of your custom scope does not hold name or oidcID for automatic group assignment through oidc provider. Please check %s", provider.Name) + errs = append(errs, &user.ErrOpenIDCustomScopeMalformed{}) + continue + } + teamData = append(teamData, models.OIDCTeamData{TeamName: name, OidcID: oidcID, Description: description}) + } + return teamData, errs +} + +func CreateTeamWithData(s *xorm.Session, teamData models.OIDCTeamData, u *user.User) (team *models.Team, err error) { + team = &models.Team{ + Name: teamData.TeamName, + Description: teamData.Description, + OidcID: teamData.OidcID, + } + err = team.Create(s, u) + return team, err +} + +// this functions creates an array of existing teams that was generated from the oidc data. +func GetOrCreateTeamsByOIDCAndNames(s *xorm.Session, teamData []models.OIDCTeamData, u *user.User) (te []*models.Team, err error) { + te = []*models.Team{} + // Procedure can only be successful if oidcID is set + for _, oidcTeam := range teamData { + team, err := models.GetTeamByOidcIDAndName(s, oidcTeam.OidcID, oidcTeam.TeamName) + if err != nil { + log.Debugf("Team with oidc_id %v and name %v does not exist. Creating team.. ", oidcTeam.OidcID, oidcTeam.TeamName) + newTeam, err := CreateTeamWithData(s, oidcTeam, u) + if err != nil { + return te, err + } + te = append(te, newTeam) + } else { + log.Debugf("Team with oidc_id %v and name %v already exists.", team.OidcID, team.Name) + te = append(te, team) + } + } + return te, err +} + func getOrCreateUser(s *xorm.Session, cl *claims, issuer, subject string) (u *user.User, err error) { + // Check if the user exists for that issuer and subject u, err = user.GetUserWithEmail(s, &user.User{ Issuer: issuer, diff --git a/pkg/modules/auth/openid/openid_test.go b/pkg/modules/auth/openid/openid_test.go index 18da67eb7..ea9b6fa22 100644 --- a/pkg/modules/auth/openid/openid_test.go +++ b/pkg/modules/auth/openid/openid_test.go @@ -20,7 +20,9 @@ import ( "testing" "code.vikunja.io/api/pkg/db" - + "code.vikunja.io/api/pkg/models" + "code.vikunja.io/api/pkg/user" + "code.vikunja.io/api/pkg/utils" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -95,4 +97,145 @@ func TestGetOrCreateUser(t *testing.T) { "email": cl.Email, }, false) }) + t.Run("existing user, non existing team", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + team := "new sso team" + oidcID := "47404" + cl := &claims{ + Email: "other-email-address@some.service.com", + VikunjaGroups: []map[string]interface{}{ + {"name": team, "oidcID": oidcID}, + }, + } + + u, err := getOrCreateUser(s, cl, "https://some.service.com", "12345") + require.NoError(t, err) + teamData, errs := getTeamDataFromToken(cl.VikunjaGroups, nil) + for _, err := range errs { + require.NoError(t, err) + } + require.NoError(t, err) + oidcTeams, err := AssignOrCreateUserToTeams(s, u, teamData) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + db.AssertExists(t, "users", map[string]interface{}{ + "id": u.ID, + "email": cl.Email, + }, false) + db.AssertExists(t, "teams", map[string]interface{}{ + "id": oidcTeams, + "name": team, + }, false) + }) + + t.Run("existing user, assign to existing team", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + team := "testteam14" + oidcID := "14" + cl := &claims{ + Email: "other-email-address@some.service.com", + VikunjaGroups: []map[string]interface{}{ + {"name": team, "oidcID": oidcID}, + }, + } + + u := &user.User{ID: 10} + teamData, errs := getTeamDataFromToken(cl.VikunjaGroups, nil) + for _, err := range errs { + require.NoError(t, err) + } + oidcTeams, err := AssignOrCreateUserToTeams(s, u, teamData) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + db.AssertExists(t, "team_members", map[string]interface{}{ + "team_id": oidcTeams, + "user_id": u.ID, + }, false) + }) + t.Run("existing user, remove from existing team", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + cl := &claims{ + Email: "other-email-address@some.service.com", + VikunjaGroups: []map[string]interface{}{}, + } + + u := &user.User{ID: 10} + teamData, errs := getTeamDataFromToken(cl.VikunjaGroups, nil) + if len(errs) > 0 { + for _, err := range errs { + require.NoError(t, err) + } + } + oldOidcTeams, err := models.FindAllOidcTeamIDsForUser(s, u.ID) + require.NoError(t, err) + oidcTeams, err := AssignOrCreateUserToTeams(s, u, teamData) + require.NoError(t, err) + teamIDsToLeave := utils.NotIn(oldOidcTeams, oidcTeams) + require.NoError(t, err) + err = RemoveUserFromTeamsByIds(s, u, teamIDsToLeave) + require.NoError(t, err) + errs = RemoveEmptySSOTeams(s, teamIDsToLeave) + for _, err = range errs { + require.NoError(t, err) + } + errs = RemoveEmptySSOTeams(s, teamIDsToLeave) + for _, err = range errs { + require.NoError(t, err) + } + err = s.Commit() + require.NoError(t, err) + + db.AssertMissing(t, "team_members", map[string]interface{}{ + "team_id": oidcTeams, + "user_id": u.ID, + }) + }) + t.Run("existing user, remove from existing team and delete team", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + cl := &claims{ + Email: "other-email-address@some.service.com", + VikunjaGroups: []map[string]interface{}{}, + } + + u := &user.User{ID: 10} + teamData, errs := getTeamDataFromToken(cl.VikunjaGroups, nil) + if len(errs) > 0 { + for _, err := range errs { + require.NoError(t, err) + } + } + oldOidcTeams, err := models.FindAllOidcTeamIDsForUser(s, u.ID) + require.NoError(t, err) + oidcTeams, err := AssignOrCreateUserToTeams(s, u, teamData) + require.NoError(t, err) + teamIDsToLeave := utils.NotIn(oldOidcTeams, oidcTeams) + require.NoError(t, err) + err = RemoveUserFromTeamsByIds(s, u, teamIDsToLeave) + require.NoError(t, err) + errs = RemoveEmptySSOTeams(s, teamIDsToLeave) + for _, err := range errs { + require.NoError(t, err) + } + err = s.Commit() + require.NoError(t, err) + db.AssertMissing(t, "teams", map[string]interface{}{ + "id": oidcTeams, + }) + }) } diff --git a/pkg/modules/auth/openid/providers.go b/pkg/modules/auth/openid/providers.go index 8791491f5..5e14c1b31 100644 --- a/pkg/modules/auth/openid/providers.go +++ b/pkg/modules/auth/openid/providers.go @@ -125,6 +125,10 @@ func getProviderFromMap(pi map[string]interface{}) (provider *Provider, err erro logoutURL = "" } + scope, _ := pi["scope"].(string) + if scope == "" { + scope = "openid profile email" + } provider = &Provider{ Name: pi["name"].(string), Key: k, @@ -132,6 +136,7 @@ func getProviderFromMap(pi map[string]interface{}) (provider *Provider, err erro OriginalAuthURL: pi["authurl"].(string), ClientSecret: pi["clientsecret"].(string), LogoutURL: logoutURL, + Scope: scope, } cl, is := pi["clientid"].(int) diff --git a/pkg/swagger/docs.go b/pkg/swagger/docs.go index 590a4e7ca..59123e5d4 100644 --- a/pkg/swagger/docs.go +++ b/pkg/swagger/docs.go @@ -8300,6 +8300,11 @@ const docTemplate = `{ "maxLength": 250, "minLength": 1 }, + "oidc_id": { + "description": "The team's oidc id delivered by the oidc provider", + "type": "string", + "maxLength": 250 + }, "updated": { "description": "A timestamp when this relation was last updated. You cannot change this value.", "type": "string" @@ -8430,6 +8435,11 @@ const docTemplate = `{ "maxLength": 250, "minLength": 1 }, + "oidc_id": { + "description": "The team's oidc id delivered by the oidc provider", + "type": "string", + "maxLength": 250 + }, "right": { "$ref": "#/definitions/models.Right" }, @@ -8573,6 +8583,9 @@ const docTemplate = `{ }, "name": { "type": "string" + }, + "scope": { + "type": "string" } } }, diff --git a/pkg/swagger/swagger.json b/pkg/swagger/swagger.json index d03f3a34a..dcb4d07d9 100644 --- a/pkg/swagger/swagger.json +++ b/pkg/swagger/swagger.json @@ -8292,6 +8292,11 @@ "maxLength": 250, "minLength": 1 }, + "oidc_id": { + "description": "The team's oidc id delivered by the oidc provider", + "type": "string", + "maxLength": 250 + }, "updated": { "description": "A timestamp when this relation was last updated. You cannot change this value.", "type": "string" @@ -8422,6 +8427,11 @@ "maxLength": 250, "minLength": 1 }, + "oidc_id": { + "description": "The team's oidc id delivered by the oidc provider", + "type": "string", + "maxLength": 250 + }, "right": { "$ref": "#/definitions/models.Right" }, @@ -8565,6 +8575,9 @@ }, "name": { "type": "string" + }, + "scope": { + "type": "string" } } }, diff --git a/pkg/swagger/swagger.yaml b/pkg/swagger/swagger.yaml index 32bbdada6..08eeb64f7 100644 --- a/pkg/swagger/swagger.yaml +++ b/pkg/swagger/swagger.yaml @@ -904,6 +904,10 @@ definitions: maxLength: 250 minLength: 1 type: string + oidc_id: + description: The team's oidc id delivered by the oidc provider + maxLength: 250 + type: string updated: description: A timestamp when this relation was last updated. You cannot change this value. @@ -1007,6 +1011,10 @@ definitions: maxLength: 250 minLength: 1 type: string + oidc_id: + description: The team's oidc id delivered by the oidc provider + maxLength: 250 + type: string right: $ref: '#/definitions/models.Right' updated: @@ -1116,6 +1124,8 @@ definitions: type: string name: type: string + scope: + type: string type: object todoist.Migration: properties: diff --git a/pkg/user/error.go b/pkg/user/error.go index cb73f06d4..26deaa4cb 100644 --- a/pkg/user/error.go +++ b/pkg/user/error.go @@ -426,6 +426,32 @@ func (err *ErrNoOpenIDEmailProvided) HTTPError() web.HTTPError { } } +// ErrNoOpenIDEmailProvided represents a "NoEmailProvided" kind of error. +type ErrOpenIDCustomScopeMalformed struct { +} + +// IsErrNoEmailProvided checks if an error is a ErrNoOpenIDEmailProvided. +func IsErrOpenIDCustomScopeMalformed(err error) bool { + _, ok := err.(*ErrOpenIDCustomScopeMalformed) + return ok +} + +func (err *ErrOpenIDCustomScopeMalformed) Error() string { + return "Custom Scope malformed" +} + +// ErrCodeNoOpenIDEmailProvided holds the unique world-error code of this error +const ErrCodeOpenIDCustomScopeMalformed = 1022 + +// HTTPError holds the http error description +func (err *ErrOpenIDCustomScopeMalformed) HTTPError() web.HTTPError { + return web.HTTPError{ + HTTPCode: http.StatusPreconditionFailed, + Code: ErrCodeOpenIDCustomScopeMalformed, + Message: "The custom scope set by the OIDC provider is malformed. Please make sure the openid provider sets the data correctly for your scope. Check especially to have set an oidcID", + } +} + // ErrAccountDisabled represents a "AccountDisabled" kind of error. type ErrAccountDisabled struct { UserID int64 diff --git a/pkg/utils/slice_difference.go b/pkg/utils/slice_difference.go new file mode 100644 index 000000000..fa3323407 --- /dev/null +++ b/pkg/utils/slice_difference.go @@ -0,0 +1,37 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public Licensee as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public Licensee for more details. +// +// You should have received a copy of the GNU Affero General Public Licensee +// along with this program. If not, see . + +package utils + +// find the elements which appear in slice1, but not in slice2 +func NotIn(slice1 []int64, slice2 []int64) []int64 { + var diff []int64 + + for _, s1 := range slice1 { + found := false + for _, s2 := range slice2 { + if s1 == s2 { + found = true + break + } + } + // int64 not found. We add it to return slice + if !found { + diff = append(diff, s1) + } + } + return diff +}