feat: assign users to teams via OIDC claims #1393
|
@ -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:
|
||||
|
|
|
@ -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.
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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 >}}
|
||||
|
||||
viehlieb marked this conversation as resolved
Outdated
|
||||
## 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
|
||||
konrad
commented
Here it is called Here it is called `vikunja_groups` and in the text before it's `vikunja_scope`. Should this be the same?
viehlieb
commented
one is the required scope, which has to be customized -> vikunja_scope the scope attribute in vikunja should be pluralized into "scopes" in authentik the attribute that is asked for is referred to as scope. one is the required scope, which has to be customized -> vikunja_scope
another one is a key for the groups within this scope -> vikunja_groups.
the scope attribute in vikunja should be pluralized into "scopes"
in authentik the attribute that is asked for is referred to as scope.
![Screenshot from 2023-12-06 14-35-25.png
](https://kolaente.dev/vikunja/api/issues/1393/attachments/941b6b69-6f92-4abe-8d1c-b23ab43e8626)
konrad
commented
Can you add this explanation to the docs? Can you add this explanation to the docs?
viehlieb
commented
I changed the docs a little to clarify this.
https://kolaente.dev/vikunja/vikunja/commit/5f521c0982aa220b687c915cd8b02a01d3c696ab
Not sure if there is more needed.
|
||||
|
||||
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 `<your authentik url>/if/admin/#/core/property-mappings`
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Can you move the title to the outside of the example json? Can you move the title to the outside of the example json?
|
||||
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`:
|
||||
konrad marked this conversation as resolved
Outdated
konrad
commented
Please add to this guide how to add the scope to the Auth provider so that it actually shows up in Vikunja. Please add to this guide how to add the scope to the Auth provider so that it actually shows up in Vikunja.
konrad
commented
Now done. Now done.
|
||||
|
||||
```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",
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Please change this so that it's clear that we're talking about Vikunja here. Please change this so that it's clear that we're talking about Vikunja here.
|
||||
"oidcID": 33349
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
What's the What's the `XXXX` part?
|
||||
},
|
||||
{
|
||||
"name": "team 2",
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Please don't make every paragraph here bold. Please don't make every paragraph here bold.
|
||||
"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.
|
||||
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
This is unnecessary, please remove it This is unnecessary, please remove it
|
||||
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.
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
What kind of behaviour? Please rename it to something more specific. And please don't capitalize the heading, same for the other headings. What kind of behaviour? Please rename it to something more specific.
And please don't capitalize the heading, same for the other headings.
|
||||
|
||||
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
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Is the attribute called Is the attribute called `oidcId` or `oidcID`? The Authentik example was using `oidcId`.
viehlieb
commented
in authentik it is called in migration:
Therefore in the db it is called oidc_id I agree, there is an inconsistency in the .md file. I try to fit the naming to the context. in authentik it is called `oidcID`
in migration:
```
type teams20230104152903 struct {
OidcID string `xorm:"varchar(250) null" maxLength:"250" json:"oidc_id"`
}
```
Therefore in the db it is called oidc_id
I agree, there is an inconsistency in the .md file.
I try to fit the naming to the context.
Do you have suggested changes in renaming?
konrad
commented
I think the name is fine, but please use the same one everywhere. I think the name is fine, but please use the same one everywhere.
|
||||
|
||||
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.
|
|
@ -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
|
||||
|
||||
konrad marked this conversation as resolved
Outdated
konrad
commented
These error codes are duplicated with the block above. These error codes are duplicated with the block above.
konrad
commented
Done. Done.
|
||||
|
|
|
@ -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}`
|
||||
|
|
|
@ -9,6 +9,7 @@ export interface ITeam extends IAbstract {
|
|||
description: string
|
||||
members: ITeamMember[]
|
||||
right: Right
|
||||
oidcId: string
|
||||
|
||||
createdBy: IUser
|
||||
created: Date
|
||||
|
|
|
@ -13,6 +13,7 @@ export default class TeamModel extends AbstractModel<ITeam> implements ITeam {
|
|||
description = ''
|
||||
members: ITeamMember[] = []
|
||||
right: Right = RIGHTS.READ
|
||||
oidcId = ''
|
||||
|
||||
createdBy: IUser = {} // FIXME: seems wrong
|
||||
created: Date = null
|
||||
|
|
|
@ -4,4 +4,5 @@ export interface IProvider {
|
|||
authUrl: string;
|
||||
clientId: string;
|
||||
logoutUrl: string;
|
||||
scope: string;
|
||||
}
|
||||
|
|
|
@ -4,7 +4,7 @@
|
|||
:class="{ 'is-loading': teamService.loading }"
|
||||
>
|
||||
<card
|
||||
v-if="userIsAdmin"
|
||||
v-if="userIsAdmin && !team.oidcId"
|
||||
class="is-fullwidth"
|
||||
:title="title"
|
||||
>
|
||||
|
@ -77,7 +77,7 @@
|
|||
:padding="false"
|
||||
>
|
||||
<div
|
||||
v-if="userIsAdmin"
|
||||
v-if="userIsAdmin && !team.oidcId"
|
||||
class="p-4"
|
||||
>
|
||||
<div class="field has-addons">
|
||||
|
|
|
@ -17,11 +17,13 @@
|
|||
class="teams box"
|
||||
>
|
||||
<li
|
||||
v-for="team in teams"
|
||||
:key="team.id"
|
||||
v-for="t in teams"
|
||||
:key="t.id"
|
||||
>
|
||||
<router-link :to="{name: 'teams.edit', params: {id: team.id}}">
|
||||
{{ team.name }}
|
||||
<router-link :to="{name: 'teams.edit', params: {id: t.id}}">
|
||||
<p>
|
||||
{{ t.name + (t.oidcId ? ` (sso: ${t.oidcId})`: '') }}
|
||||
</p>
|
||||
</router-link>
|
||||
</li>
|
||||
</ul>
|
||||
|
|
|
@ -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"
|
||||
|
|
|
@ -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
|
|
@ -28,4 +28,8 @@
|
|||
created_by_id: 7
|
||||
- id: 13
|
||||
name: testteam13
|
||||
created_by_id: 7
|
||||
created_by_id: 7
|
||||
- id: 14
|
||||
name: testteam14
|
||||
created_by_id: 7
|
||||
oidc_id: 14
|
|
@ -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 <https://www.gnu.org/licenses/>.
|
||||
|
||||
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
|
||||
},
|
||||
})
|
||||
}
|
|
@ -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 {
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Please add a comment, similar to the other errors. Please add a comment, similar to the other errors.
|
||||
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."}
|
||||
}
|
||||
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Is this an error about one team not existing or multiple? Because the error name suggests one or more teams but the error message is about only one. Is this an error about one team not existing or multiple? Because the error name suggests one or more teams but the error message is about only one.
|
||||
// ErrOIDCTeamsDoNotExistForUser represents an error where an oidcTeam does not exist for the user
|
||||
type ErrOIDCTeamsDoNotExistForUser struct {
|
||||
UserID int64
|
||||
}
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Please add the error to the error docs. Please add the error to the error docs.
|
||||
|
||||
// IsErrOIDCTeamsDoNotExistForUser checks if an error is ErrOIDCTeamsDoNotExistForUser.
|
||||
func IsErrOIDCTeamsDoNotExistForUser(err error) bool {
|
||||
_, ok := err.(ErrOIDCTeamsDoNotExistForUser)
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Please change the message to "No team with that name exists" Please change the message to "No team with that name exists"
viehlieb
commented
done all the error stuff done all the error stuff
|
||||
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."}
|
||||
}
|
||||
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
What OIDC does not exist? What does that mean? What OIDC does not exist? What does that mean?
|
||||
// ====================
|
||||
// User <-> Project errors
|
||||
// ====================
|
||||
|
|
|
@ -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).
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Can you do this either with a join to avoid loading the user or at least use the "simple" function? Can you do this either with a join to avoid loading the user or at least use the "simple" function?
viehlieb
commented
done. done.
|
||||
Exist(&TeamMember{})
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
This should be This should be `Exist(&TeamMember{})`.
viehlieb
commented
done done
|
||||
}
|
||||
|
||||
// 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.
|
||||
|
|
|
@ -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 {
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
What is this used for? Please add a comment. What is this used for? Please add a comment.
konrad
commented
What's the difference to the What's the difference to the `Team` struct?
viehlieb
commented
Intermediate struct which only holds TeamName and OidcId Description. Intermediate struct which only holds TeamName and OidcId Description.
It does not exist as a Team yet or better: it is the data accessible via oidc, which the Team struct is compared against.
konrad
commented
Okay, but isn't it missing the json tags then? I'm not sure if I understood you correctly herre. If it's only used for oidc data the name should reflect that. Okay, but isn't it missing the json tags then? I'm not sure if I understood you correctly herre.
If it's only used for oidc data the name should reflect that. `TeamData` is too generic.
viehlieb
commented
No, the teamData has to be pulled out of token via
Called it OIDCTeamData now. No, the teamData has to be pulled out of token via
`getTeamDataFromToken`
Called it OIDCTeamData now.
|
||||
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) {
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Please return a pointer to a Please return a pointer to a
viehlieb
commented
team i suppose.done, tx team i suppose.done, tx
|
||||
team := &Team{}
|
||||
has, err := s.
|
||||
Table("teams").
|
||||
Where("oidc_id = ? AND name = ?", oidcID, teamName).
|
||||
Get(team)
|
||||
konrad marked this conversation as resolved
Outdated
konrad
commented
Did you try passing the Did you try passing the
viehlieb
commented
Ups, I guess here is something missing Ups, I guess here is something missing
|
||||
if !has || err != nil {
|
||||
return nil, ErrOIDCTeamDoesNotExist{teamName, oidcID}
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
`Get` will always return one entry, no need for `Asc` or `Limit`.
|
||||
}
|
||||
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 {
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Please handle the error. Please handle the error.
|
||||
return ts, err
|
||||
}
|
||||
return ts, nil
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
What happens if there's more than one team with that combination (unlikely, but not handled) What happens if there's more than one team with that combination (unlikely, but not handled)
viehlieb
commented
I tend to just changing functionality to take the first team with that specific setting. I tend to just changing functionality to take the first team with that specific setting.
konrad
commented
That sounds like it could work. Easiest way would be to add a That sounds like it could work. Easiest way would be to add a `OrderBy` clause here.
|
||||
}
|
||||
|
||||
func addMoreInfoToTeams(s *xorm.Session, teams []*Team) (err error) {
|
||||
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
This error message should contain the name as well. This error message should contain the name as well.
|
||||
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
|
||||
|
|
|
@ -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 {
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Didn't you declare this already somewhere else? Didn't you declare this already somewhere else?
|
||||
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)
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
The next 20 lines should be wrapped in a The next 20 lines should be wrapped in a `if teamData != nil` so that the whole thing only gets executed when there is team data in the token. Otherwise this might generate error messages for setups not using teams via oicd.
viehlieb
commented
i removed this because of "cyclomatic complexity". But i can try to put it back in. i removed this because of "cyclomatic complexity". But i can try to put it back in.
konrad
commented
Well if the cyclomatic complexity gets too high the fix would be to refactor this into multiple functions, not to remove the check :) because the check should be there. Well if the cyclomatic complexity gets too high the fix would be to refactor this into multiple functions, not to remove the check :) because the check should be there.
|
||||
}
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
I feel like the next 20 lines need better error handling - is it intended to not return when an error happens? I feel like the next 20 lines need better error handling - is it intended to not return when an error happens?
viehlieb
commented
The rationale was to ensure users can still log in, even though the scope is malformed. Also: It might be some groups with valid token details get through, some not. We can also go for not signing in altogether. The rationale was to ensure users can still log in, even though the scope is malformed.
Therefore the Errors are logged instead of returning.
Also: It might be some groups with valid token details get through, some not.
We can also go for not signing in altogether.
konrad
commented
If this works and won't create duplicate teams when users log in again in the future, I think it's fine to leave it like this. If this works and won't create duplicate teams when users log in again in the future, I think it's fine to leave it like this.
|
||||
|
||||
// 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)
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Typo: Please check the other log messages as well for typos, I've seen a few more like this. Typo: `team` and `oidc` should be lowercase.
Please check the other log messages as well for typos, I've seen a few more like this.
|
||||
}
|
||||
|
||||
//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)
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Please don't ignore the error. Please don't ignore the error.
|
||||
}
|
||||
oidcTeams, err := AssignOrCreateUserToTeams(s, u, teamData)
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Please don't call this "Sign out". That's a different thing. Please don't call this "Sign out". That's a different thing.
|
||||
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 {
|
||||
konrad marked this conversation as resolved
Outdated
konrad
commented
Doing this every time a user logs in probably has performance implications but that's a problem to care about when we'll hit it in the wild. Doing this every time a user logs in probably has performance implications but that's a problem to care about when we'll hit it in the wild.
viehlieb
commented
only if it has teamIDsToLeave, but yeah I agree. The problem is more the oidc structure itself, that you "have" to check for changes "everytime" you sign in.. only if it has teamIDsToLeave, but yeah I agree.
The problem is more the oidc structure itself, that you "have" to check for changes "everytime" you sign in..
|
||||
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)
|
||||
}
|
||||
}
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Replace with
Replace with
```
if len(teamData) < 1 {
return
}
viehlieb
commented
It could well be, that only a single team is received through the oidc token. It could well be, that only a single team is received through the oidc token.
konrad
commented
But I should have phrased it better, this is equivalent:
But `len(teamData) < 1` will only be true if there are 0 teams received?
I should have phrased it better, this is equivalent:
```
if len(teamData) == 0 {
return
}
```
viehlieb
commented
yes, that's true of course. i do not know why i read it differently. yes, that's true of course. i do not know why i read it differently.
|
||||
}
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
This should be an This should be an `Error` log message.
|
||||
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.
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Please change this error message to Please change this error message to `Could not assign user %s to team %s: %v` - we also have assignees so this could be confusing on its own.
|
||||
// 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)
|
||||
konrad marked this conversation as resolved
Outdated
konrad
commented
What does this do? Ensure the user is only member of teams provided by the oidc provider? What about teams created in Vikunja manually? I feel like we should not delete a team here as that's pretty destructive. Maybe add a command to clean up empty teams later but I think we should not do that here. What does this do? Ensure the user is only member of teams provided by the oidc provider? What about teams created in Vikunja manually?
I feel like we should not delete a team here as that's pretty destructive. Maybe add a command to clean up empty teams later but I think we should not do that here.
viehlieb
commented
User as "team_member" will be removed via Further explanation below: deleting the team is not managed via oidc. Meaning:
what is missing though: User as "team_member" will be removed via `(user.id, team.id)` for all `teamIDs []int64`.
If the user is the last member, the team is deleted.
Further explanation below:
deleting the team is not managed via oidc.
the token delivered by the oidc provider just contains information about the team memberships of the user.
### Meaning:
* `team1` exists and was created through oidc (oidcId is set to something not null or "").
* `user1` is team_member of team1.
* changes in oidc provider change team_membership of `user1` >> `team1` is not longer contained in oidc token for `user1`.
* `user1` logs in through oidc and is kicked out of team, which is not possible, if `user1` is the last member of `team1`.
* if `user1` is last member >> `team1` is deleted.
what is missing though:
a check whether `user1` is in fact the last member of `team1`, then only delete `team1`.
konrad
commented
Ah, now I understand how that came about. I still think we should not delete the team at that moment, better change the function to allow removing the last member in that case. There will probably be cases where a user is accidentally removed from the oicd claim, the team gets deleted and all shares with that team as well. Then the user is added again, and the shares need to be added again. It's fine to use this to sync teams and memberships but I feel like there's more to it. > user1 logs in through oidc and is kicked out of team, which is not possible, if user1 is the last member of team1.
Ah, now I understand how that came about. I still think we should not delete the team at that moment, better change the function to allow removing the last member in that case. There will probably be cases where a user is accidentally removed from the oicd claim, the team gets deleted and all shares with that team as well. Then the user is added again, and the shares need to be added again.
I don't know if this is only hypothetical, but it's something users will complain about when it happens.
It's fine to use this to sync teams and memberships but I feel like there's more to it.
viehlieb
commented
okay, teams won't be deleted okay, teams won't be deleted
|
||||
return nil, err
|
||||
}
|
||||
for _, team := range teams {
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Please call this something like Please call this something like `RemoveUserFromTeamsByID` so that it is clear how destructive this is.
|
||||
tm := models.TeamMember{TeamID: team.ID, UserID: u.ID, Username: u.Username}
|
||||
exists, _ := tm.MembershipExists(s)
|
||||
if !exists {
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
why not do this in one sql query? why not do this in one sql query?
viehlieb
commented
You mean something like:
You mean something like:
```
func RemoveUserFromTeamsByIds(s *xorm.Session, u *user.User, teamIDs []int64) (err error) {
if len(teamIDs) < 1 {
return nil
}
strSlice := make([]string, len(teamIDs))
for i, num := range teamIDs {
strSlice[i] = strconv.FormatInt(num, 10)
}
sql := fmt.Sprintf(`DELETE FROM team_members WHERE user_id = %d AND team_id IN (%v)`, u.ID, strings.Join(strSlice, ","))
_, err = s.Exec(sql)
return err
```
konrad
commented
Yes, but with xorm:
Yes, but with xorm:
```
_, err = s.Where(builder.And(builder.Where("user_id = ?", u.ID), builder.In("team_id", teamIDs)).Delete(&TeamMember{})
return err
```
viehlieb
commented
went for:
went for:
`_, err = s.Where("team_id IN (?) AND user_id = ?", strings.Join(strSlice, ","), u.ID).Delete(&models.TeamMember{})`
konrad
commented
That's still working agains the orm - makes things more complicated in the end. Please use xorm's Something like this might work as well:
That's still working agains the orm - makes things more complicated in the end.
Please use xorm's `In` function instead.
Something like this might work as well:
```go
_, err = s.And("user_id = ?", u.ID).In("team_id", teamIDs).Delete(&TeamMember{})
return err
```
|
||||
err = tm.Create(s, u)
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Please return the error instead of logging only. Please return the error instead of logging only.
konrad
commented
Please don't ignore the errors. Please don't ignore the errors.
viehlieb
commented
not ignored now not ignored now
|
||||
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
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
This whole inner loop should be rewritten to something like this so that it's more idomatic go:
This whole inner loop should be rewritten to something like this so that it's more idomatic go:
```go
tm := models.TeamMember{TeamID: teamID, Username: u.Username}
err := tm.Delete(s, u)
if err != nil {
return err
}
team, err := models.GetTeamByID(s, teamID)
if err != nil {
return err
}
err = team.Delete(s, u)
if err != nil {
return err
}
konrad
commented
Also right now you'll only delete the team if deleting the member did not work, which does not really make sense. Also right now you'll only delete the team if deleting the member did not work, which does not really make sense.
viehlieb
commented
changed behaviour to not delete team changed behaviour to not delete team
|
||||
}
|
||||
|
||||
func RemoveEmptySSOTeams(s *xorm.Session, teamIDs []int64) (errs []error) {
|
||||
for _, teamID := range teamIDs {
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Not sure if this is a good solution. Imagine one membership fails to get removed while the others get through. Then there's inconsistent state and the whole thing is not atomic anymore. I think we should instead return on the first error and let the caller deal with it. Not sure if this is a good solution. Imagine one membership fails to get removed while the others get through. Then there's inconsistent state and the whole thing is not atomic anymore. I think we should instead return on the first error and let the caller deal with it.
viehlieb
commented
okay, but won't it reach inconsistent state anyways? or do you suggest to catch ErrCannotDeleteLastTeamMember and ask to sign out manually from that specific group(s) and sign out from other groups anyway? I think it is intended behavior to sign out from teams when possible, and to leave teams as they are, where error is found. Maybe giving a hint to leave manually because you are last user. Did I understand you correctly? okay, but won't it reach inconsistent state anyways?
Imagine the user is in 3 groups.
tm is deleted for group 1
tm is not deleted for group 2, because user is last member
tm won't be deleted for group 3, where user is not last member and should be removed.
or do you suggest to catch ErrCannotDeleteLastTeamMember and ask to sign out manually from that specific group(s) and sign out from other groups anyway?
I think it is intended behavior to sign out from teams when possible, and to leave teams as they are, where error is found. Maybe giving a hint to leave manually because you are last user.
Did I understand you correctly?
konrad
commented
What's your difference between "signing out from a team" and "leaving a team"? Aren't the two the same?
Not if the whole db transaction is rolled back. > I think it is intended behavior to sign out from teams when possible, and to leave teams, where error is found.
What's your difference between "signing out from a team" and "leaving a team"? Aren't the two the same?
> okay, but won't it reach inconsistent state anyways?
Not if the whole db transaction is rolled back.
viehlieb
commented
edited edited
viehlieb
commented
the edit is:
I think it is intended behavior to sign out from teams when possible, and to leave teams as they are, where error is found. Maybe giving a hint to leave manually because you are last user.
If the whole transaction is rolled back, then it might very challening to organize teams, because if a member sits on a team as last member, then the member cannot leave this team. the edit is:
> > I think it is intended behavior to sign out from teams when possible, and to leave teams, where error is found.
>
> What's your difference between "signing out from a team" and "leaving a team"? Aren't the two the same?
>
I think it is intended behavior to sign out from teams when possible, and to leave teams **as they are**, where error is found. Maybe giving a hint to leave manually because you are last user.
> > okay, but won't it reach inconsistent state anyways?
> Not if the whole db transaction is rolled back.
If the whole transaction is rolled back, then it might very challening to organize teams, because if a member sits on a team as last member, then the member **cannot leave this** team.
The suggestion would be to leave teams, if it is possible -> the team membership, where the user cannot be signed out because of
```ErrCannotDeleteLastTeamMember{tm.TeamID, tm.UserID}```
should be resolved manually then
konrad
commented
Ah so "leave a team" in this context is "not deleting / modifying it"? Not "A user leaves a team"? I guess I'm just understanding it differently :)
Why would it do that? That's not what should happen, but how would being the last member of one team prevent that user to leave another team?
But resolving this manually would mean the user needs to manually remove the team, which they cannot do because the team was created from openid? > to leave teams as they are, where error is found
Ah so "leave a team" in this context is "not deleting / modifying it"? Not "A user leaves a team"?
I guess I'm just understanding it differently :)
> if a member sits on a team as last member, then the member cannot leave other teams.
Why would it do that? That's not what should happen, but how would being the last member of one team prevent that user to leave another team?
> where the user cannot be signed out because of
ErrCannotDeleteLastTeamMember{tm.TeamID, tm.UserID}
should be resolved manually then
But resolving this manually would mean the user needs to manually remove the team, which they cannot do because the team was created from openid?
viehlieb
commented
I think it is the responsibility of oidc admins to not let a team go blank without coordinating the potential destructive danger of it. > if a member sits on a team as last member, then the member cannot leave other teams.
...is edited. I meant: the member cannot leave this team, if it is the last membership.
> where the user cannot be signed out because of
ErrCannotDeleteLastTeamMember{tm.TeamID, tm.UserID}
should be resolved manually then
> But resolving this manually would mean the user needs to manually remove the team, which they cannot do because the team was created from openid?
I think it is the responsibility of _oidc admins_ to not let a team go blank without coordinating the potential destructive danger of it.
Therefore i would just delete the team + team_memberships.
konrad
commented
Okay, that makes more sense. > I think it is the responsibility of oidc admins to not let a team go blank without coordinating the potential destructive danger of it. Therefore i would just delete the team + team_memberships.
Okay, that makes more sense.
|
||||
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)
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Why not cast this to Why not cast this to `[]map[string]interface{}` directly?
viehlieb
commented
no reason, no reason,
had claim VikunjaGroups more agnostic to specific data type.
now casts directly to []map[string]interface{}
|
||||
_, _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
|
||||
}
|
||||
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Please use `strconv.FormatInt Please use `strconv.FormatInt
viehlieb
commented
done done
|
||||
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
|
||||
}
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Please use xorm's Please use xorm's `In` function because that allows you to pass an int slice directly and avoids the need for the conversion you're doing.
|
||||
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Please use Please use `strconv.FormatFloat` (not 100% sure if the function is really called that)
|
||||
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
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
The return here will mean if one team claim is invalid, it will not try to validate any others. Is that what you intended? The return here will mean if one team claim is invalid, it will not try to validate any others. Is that what you intended?
viehlieb
commented
changed behaviour to skipping the invalid team changed behaviour to skipping the invalid team
|
||||
_, exists := team["name"]
|
||||
if exists {
|
||||
name = team["name"].(string)
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Please use Please use `_, exists := team["name"]` to check if the field exists (`exists` is a boolean indicating whether the field exists). Same for the other fields.
|
||||
}
|
||||
_, 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)
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Please rename this to Please rename this to `team`.
konrad
commented
What's the advantage of this method over What's the advantage of this method over `team.Create`?
viehlieb
commented
i think it was cyclomatic complexity. i think it was cyclomatic complexity.
|
||||
case string:
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Please make this debug message more descriptive. Please make this debug message more descriptive.
|
||||
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 == "" {
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Please use Please use
viehlieb
commented
Here there is the information missing, what do you suggest? Here there is the information missing, what do you suggest?
konrad
commented
Sorry, I meant Sorry, I meant `[]*models.Team{}`.
|
||||
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
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Which oidcID? In the Which oidcID? In the `teamData`? Isn't that always a string, due to the type system? Then the comment is redundant, please remove or clarify further.
|
||||
}
|
||||
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
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Please make this debug message more descriptive or remove it alltogether. Please make this debug message more descriptive or remove it alltogether.
|
||||
}
|
||||
|
||||
// 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,
|
||||
|
|
|
@ -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,
|
||||
})
|
||||
})
|
||||
}
|
||||
|
|
|
@ -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,
|
||||
konrad
commented
Aren't we always expecting the scope to be Aren't we always expecting the scope to be `vikunja_groups`? Why make it configurable then instead of always appending it?
viehlieb
commented
If you have a look at:
You'll see the hardcoded scope. There was a PR on the frontend explaining this issue. This part makes sure there is always the "openid profile email" scope set - as it was before, hardcoded.. But if you want to add a custom scope in the config.yml, then you have to tell the provider in back and frontend to actually use it.
If you have a look at:
https://kolaente.dev/vikunja/frontend/src/commit/3643ffe0d0357c89cb3517fafbb0c438188ac88d/src/helpers/redirectToProvider.ts#L18
You'll see the hardcoded scope.
But to receive more information instead of openid profile email from the oidc provider you'll need an extra scope.
There was a PR on the frontend explaining this issue.
https://kolaente.dev/vikunja/frontend/pulls/2749
This part makes sure there is always the "openid profile email" scope set - as it was before, hardcoded.. But if you want to add a custom scope in the config.yml, then you have to tell the provider in back and frontend to actually use it.
```
scope, _ := pi["scope"].(string)
if scope == "" {
scope = "openid profile email"
}
```
konrad
commented
Yes it is hardcoded in the frontend, but why make it a manual config setting instead of passing it automatically from the api to the provider? That would allow us to get rid of the extra config variable. Yes it is hardcoded in the frontend, but why make it a manual config setting instead of passing it automatically from the api to the provider? That would allow us to get rid of the extra config variable.
viehlieb
commented
So you suggest to add: Anyway it has to be explained what "vikunja_scope" is and how it is used. I think it might be helpful for admins to use the scopes explicitly. So you suggest to add:
`Scope: "openid profile email vikunja_scope"`
Anyway it has to be explained what "vikunja_scope" is and how it is used. I think it might be helpful for admins to use the scopes explicitly.
But I do not have a strong opinion on that
konrad
commented
Okay, now I got you. Sorry for the confusion. Let's keep it the way you intended, please add an example scope to the Okay, now I got you. Sorry for the confusion. Let's keep it the way you intended, please add an example scope to the `config.yml.sample` file.
viehlieb
commented
Done, it is also linked to the explanation in openid.md Done, it is also linked to the explanation in openid.md
|
||||
}
|
||||
|
||||
cl, is := pi["clientid"].(int)
|
||||
|
|
|
@ -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"
|
||||
}
|
||||
}
|
||||
},
|
||||
|
|
|
@ -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"
|
||||
}
|
||||
}
|
||||
},
|
||||
|
|
|
@ -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:
|
||||
|
|
|
@ -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
|
||||
viehlieb marked this conversation as resolved
Outdated
konrad
commented
Please add this error to the error doc Please add this error to the error doc
|
||||
|
||||
// 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
|
||||
|
|
|
@ -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 <https://www.gnu.org/licenses/>.
|
||||
|
||||
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
|
||||
}
|
Typo: Should be
Authentik
.