vikunja
/
vikunja
6
43
Fork 71
Code Issues 150 Pull Requests 15 Packages Releases 32 Activity

fix: OIDC team comparison should not include the team name #2152

Merged
konrad merged 5 commits from waza-ari/vikunja:fix-team-creation into main 2024-03-04 20:26:47 +00:00
Conversation 19 Commits 5 Files Changed 6 +68 -21
waza-ari commented 2024-03-02 17:41:04 +00:00
Contributor
Copy Link

The change introduced in #2150 introduces a bug where a Team would be re-created every time a user logs in, as the check if a team already exists was based on both the unique oidcID and the name. This PR proposes to only base the check on the ID, as this should be unique.

The change introduced in #2150 introduces a bug where a Team would be re-created every time a user logs in, as the check if a team already exists was based on both the unique `oidcID` and the `name`. This PR proposes to only base the check on the ID, as this should be unique.
waza-ari added 1 commit 2024-03-02 17:41:06 +00:00
frederick commented 2024-03-02 17:50:38 +00:00
Member
Copy Link

Hi waza-ari!

Thank you for creating a PR!

I've deployed the frontend changes of this PR on a preview environment under this URL: https://2152-fix-team-creation--vikunja-frontend-preview.netlify.app

You can use this url to view the changes live and test them out.
You will need to manually connect this to an api running somewhere. The easiest to use is https://try.vikunja.io/.

This preview does not contain any changes made to the api, only the frontend.

Have a nice day!

Beep boop, I'm a bot.

Hi waza-ari! Thank you for creating a PR! I've deployed the frontend changes of this PR on a preview environment under this URL: https://2152-fix-team-creation--vikunja-frontend-preview.netlify.app You can use this url to view the changes live and test them out. You will need to manually connect this to an api running somewhere. The easiest to use is https://try.vikunja.io/. This preview does not contain any changes made to the api, only the frontend. Have a nice day! > Beep boop, I'm a bot.
waza-ari requested review from konrad 2024-03-03 12:42:57 +00:00
konrad requested changes 2024-03-03 21:31:04 +00:00
Dismissed
pkg/models/teams.go Outdated
@ -133,2 +132,3 @@
// GetTeamByOidcID returns a team matching the given oidc_id
// For oidc team creation oidcID and Name need to be set
func GetTeamByOidcIDAndName(s *xorm.Session, oidcID string, teamName string) (*Team, error) {
func GetTeamByOidcID(s *xorm.Session, oidcID string, teamName string) (*Team, error) {
konrad commented 2024-03-03 21:31:01 +00:00
Owner
Copy Link

Please remove the teamName parameter as well since it is now unused.

Please remove the `teamName` parameter as well since it is now unused.
waza-ari commented 2024-03-03 21:51:27 +00:00
Author
Contributor
Copy Link

I left it in there because it's currently passed to the ErrOIDCTeamDoesNotExist exception, and then used in various places in the code. If you prefer I can remove those occurrences, although it might be a better idea to do that together with your suggestion below

I left it in there because it's currently passed to the `ErrOIDCTeamDoesNotExist` exception, and then used in various places in the code. If you prefer I can remove those occurrences, although it might be a better idea to do that together with your suggestion below
konrad commented 2024-03-03 21:57:01 +00:00
Owner
Copy Link

I think having the name in there doesn't hold value, but having the openid provider has. Probably best to combine this with adding the combination of the provider.

I think having the name in there doesn't hold value, but having the openid provider has. Probably best to combine this with adding the combination of the provider.
waza-ari marked this conversation as resolved
konrad commented 2024-03-03 21:32:29 +00:00
Owner
Copy Link

Well I just noticed the oidc id is not enough, it should actually also use the openid provider id as well… since the id is only guaranteed to be unique within that one provider. We should merge this PR before fixing that though.

Well I just noticed the oidc id is not enough, it should actually also use the openid provider id as well… since the id is only guaranteed to be unique within that one provider. We should merge this PR before fixing that though.
waza-ari commented 2024-03-03 21:54:10 +00:00
Author
Contributor
Copy Link

Well I just noticed the oidc id is not enough, it should actually also use the openid provider id as well… since the id is only guaranteed to be unique within that one provider. We should merge this PR before fixing that though.

That is a very good point, you're absolutely right. I can try to tackle this tomorrow, question is if we first want to merge this one or not.

> Well I just noticed the oidc id is not enough, it should actually also use the openid provider id as well… since the id is only guaranteed to be unique within that one provider. We should merge this PR before fixing that though. That is a very good point, you're absolutely right. I can try to tackle this tomorrow, question is if we first want to merge this one or not.
konrad commented 2024-03-03 21:56:01 +00:00
Owner
Copy Link

That is a very good point, you're absolutely right. I can try to tackle this tomorrow, question is if we first want to merge this one or not.

Given this whole team sync feature is not released yet, we are in no rush to fix this quickly. If you plan to implement this, I'm fine waiting.

> That is a very good point, you're absolutely right. I can try to tackle this tomorrow, question is if we first want to merge this one or not. Given this whole team sync feature is not released yet, we are in no rush to fix this quickly. If you plan to implement this, I'm fine waiting.
konrad commented 2024-03-03 21:59:38 +00:00
Owner
Copy Link

Some implementation notes:

  • The provider id should be the issuer from the token, similar to when a new user is created in pkg/modules/auth/openid/openid.go:198.
  • This needs a new colum openid_issuer or something like that. Docs for creating migrations here.
Some implementation notes: * The provider id should be the issuer from the token, similar to when a new user is created in `pkg/modules/auth/openid/openid.go:198`. * This needs a new colum `openid_issuer` or something like that. [Docs for creating migrations here](https://vikunja.io/docs/database-migrations/#generating-a-new-migration-stub).
waza-ari added 11 commits 2024-03-04 13:55:10 +00:00
638f92a0cd feat(subscription): use a recursive cte to fetch subscriptions of parent projects 
Testing this locally resulted in improved response times from ~50ms to ~20ms when creating a project. It looks like even though the code running these sql queries uses different go routines, they affect each other (caused by IO or context switching?)
c822bc6c2b docs: improve OpenID documentation (#2151) 
This PR stems from issue #2150, in turn following up on PR #1393

It adds additional details around the OIDC authentication feature, as well as details about how the team assignment works.

Co-authored-by: Daniel Herrmann <daniel.herrmann1@gmail.com>
Reviewed-on: #2151
Reviewed-by: konrad <k@knt.li>
Co-authored-by: waza-ari <daniel.herrmann@makerspace-darmstadt.de>
Co-committed-by: waza-ari <daniel.herrmann@makerspace-darmstadt.de>
continuous-integration/drone/pr Build is passing Details
221359bb2e fix(deps): update dependency vue-i18n to v9.10.1
waza-ari commented 2024-03-04 15:14:47 +00:00
Author
Contributor
Copy Link

@konrad I'll work on that today, I believe it should be reasonably straight forward. One item for discussion though:

Right now, whenever a Team is created, the user who logged in and triggered the creation becomes admin of that team. Effectively this means that whoever logs in first triggering the team creation, gets admin privileges for the team. This might not be desired.

Should we extend the OIDC structure to include some admin flag?

{
    "vikunja_groups": [
        {
            "name": "team 1",
            "oidcID": 33349,
            "isAdmin": true
        },
        {
            "name": "team 2",
            "oidcID": 35933,
            "isAdmin": false
        }
    ]
}
@konrad I'll work on that today, I believe it should be reasonably straight forward. One item for discussion though: Right now, whenever a Team is created, the user who logged in and triggered the creation becomes admin of that team. Effectively this means that whoever logs in first triggering the team creation, gets admin privileges for the team. This might not be desired. Should we extend the OIDC structure to include some admin flag? ``` { "vikunja_groups": [ { "name": "team 1", "oidcID": 33349, "isAdmin": true }, { "name": "team 2", "oidcID": 35933, "isAdmin": false } ] } ```
konrad commented 2024-03-04 15:17:23 +00:00
Owner
Copy Link

Having the admin status on such a team doesn't have any benefit because the team should not be managable in Vikunja at all. This should be changed so that all users are members only, without admins (not sure how much effort that is). Could be a follow-up PR though.

Having the admin status on such a team doesn't have any benefit because the team should not be managable in Vikunja at all. This should be changed so that all users are members only, without admins (not sure how much effort that is). Could be a follow-up PR though.
👍 1
waza-ari added 1 commit 2024-03-04 16:30:42 +00:00
continuous-integration/drone/pr Build is passing Details
3a609d4d89 fix: OIDC team IDs should be bound to an issuer 
This commit introduces changes to make sure that (a) the Team name is no longer used when comparing teams for existence and (b) the issuer if stored instead, as the oidcID is only guaranteed to be unique within the same provider
waza-ari force-pushed fix-team-creation from 3a609d4d89 to 9a20596bb0 2024-03-04 16:31:18 +00:00 Compare
waza-ari force-pushed fix-team-creation from 9a20596bb0 to ea04c6cbe3 2024-03-04 16:52:11 +00:00 Compare
waza-ari commented 2024-03-04 16:55:42 +00:00
Author
Contributor
Copy Link

@konrad thanks, you're absolutely right. The PR should work as is, avoiding duplicate teams being created and leveraging the issuer to compare against existing teams.

I've not found an easy way to avoid setting the admin flag to true, as its currently done within the Teams Create method, and simply adding another parameter with default value doesn't seem to be a thing in Go :D

@konrad thanks, you're absolutely right. The PR should work as is, avoiding duplicate teams being created and leveraging the issuer to compare against existing teams. I've not found an easy way to avoid setting the admin flag to true, as its currently done within the Teams Create method, and simply adding another parameter with default value doesn't seem to be a thing in Go :D
konrad reviewed 2024-03-04 17:15:30 +00:00
pkg/models/error.go Outdated
@ -1192,3 +1192,3 @@
// 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)
return fmt.Sprintf("No team could be found for the given oidcId and issuer. [OidcID : %v] [Issuer Name: %v] ", err.OidcID, err.Issuer)
konrad commented 2024-03-04 17:14:29 +00:00
Owner
Copy Link

Nitpick: That's not the issuer name but the issuer id.

Nitpick: That's not the issuer _name_ but the issuer _id_.
waza-ari marked this conversation as resolved
konrad commented 2024-03-04 17:19:51 +00:00
Owner
Copy Link

It looks like the issuer is not set for old teams. I propose two solutions:

  1. Update the issuer when the team is updated (for the name)
  2. Delete all openid teams in the migration so that they are properly recreated when the user logs in again.
It looks like the issuer is not set for old teams. I propose two solutions: 1. Update the issuer when the team is updated (for the name) 2. Delete all openid teams in the migration so that they are properly recreated when the user logs in again.
waza-ari added 1 commit 2024-03-04 17:21:42 +00:00
continuous-integration/drone/pr Build is passing Details
3bd427c870 fix: wording in error message
waza-ari commented 2024-03-04 17:26:07 +00:00
Author
Contributor
Copy Link

It looks like the issuer is not set for old teams. I propose two solutions:

  1. Update the issuer when the team is updated (for the name)
  2. Delete all openid teams in the migration so that they are properly recreated when the user logs in again.

Hm, that's a difficult one. The first approach will likely not work, as the issuer is used to find the team in the first place. We'd need to put in extra logic to check if there's already a team with the same oidcID but no issuer set. There's a small risk that someone's already using the unstable version with two providers though.

2 is probably possible, although I may need help with that. My Go skills are very limited

Any preferences on the approach from your perspective? Maybe also the question: old teams can only exist since the initial merge a couple of days ago, is that a real problem?

> It looks like the issuer is not set for old teams. I propose two solutions: > > 1. Update the issuer when the team is updated (for the name) > 2. Delete all openid teams in the migration so that they are properly recreated when the user logs in again. Hm, that's a difficult one. The first approach will likely not work, as the issuer is used to find the team in the first place. We'd need to put in extra logic to check if there's already a team with the same oidcID but no issuer set. There's a small risk that someone's already using the unstable version with two providers though. 2 is probably possible, although I may need help with that. My Go skills are very limited Any preferences on the approach from your perspective? Maybe also the question: old teams can only exist since the initial merge a couple of days ago, is that a real problem?
konrad commented 2024-03-04 17:36:48 +00:00
Owner
Copy Link

Any preferences on the approach from your perspective? Maybe also the question: old teams can only exist since the initial merge a couple of days ago, is that a real problem?

It's probably not a real problem. Maybe best to just ignore the problem and let users who are already using this (probably us two) delete the old teams manually with a delete from teams where issuer is null.

I've noticed another odd thing in my own testing: Even though I added the vikunja_groups scope and mapping, Authentik does not return it properly. It does, however, return a groups claim in the token with all groups the user is part of. Testing with GitLab, it returns a groups_direct property in the token with all GitLab organizations I am part of. Not sure where that's coming from and where my own custom mapping went. Will investigate this more.

I've not found an easy way to avoid setting the admin flag to true, as its currently done within the Teams Create method, and simply adding another parameter with default value doesn't seem to be a thing in Go :D

What about setting it to false? Where did you see the admin value being set?
(As I said, this could be a follow-up)

> Any preferences on the approach from your perspective? Maybe also the question: old teams can only exist since the initial merge a couple of days ago, is that a real problem? It's probably not a real problem. Maybe best to just ignore the problem and let users who are already using this (probably us two) delete the old teams manually with a `delete from teams where issuer is null`. --- I've noticed another odd thing in my own testing: Even though I added the `vikunja_groups` scope and mapping, Authentik does not return it properly. It does, however, return a `groups` claim in the token with all groups the user is part of. Testing with GitLab, it returns a `groups_direct` property in the token with all GitLab organizations I am part of. Not sure where that's coming from and where my own custom mapping went. Will investigate this more. --- > I've not found an easy way to avoid setting the admin flag to true, as its currently done within the Teams Create method, and simply adding another parameter with default value doesn't seem to be a thing in Go :D What about setting it to false? Where did you see the admin value being set? (As I said, this could be a follow-up)
konrad commented 2024-03-04 17:46:38 +00:00
Owner
Copy Link

I've noticed another odd thing in my own testing: Even though I added the vikunja_groups scope and mapping, Authentik does not return it properly. It does, however, return a groups claim in the token with all groups the user is part of. Testing with GitLab, it returns a groups_direct property in the token with all GitLab organizations I am part of. Not sure where that's coming from and where my own custom mapping went. Will investigate this more.

Well that was a false alarm because I didn't rebuild my frontend and therefore it didn't pass the scopes back to Authentik when redirecting.

> I've noticed another odd thing in my own testing: Even though I added the vikunja_groups scope and mapping, Authentik does not return it properly. It does, however, return a groups claim in the token with all groups the user is part of. Testing with GitLab, it returns a groups_direct property in the token with all GitLab organizations I am part of. Not sure where that's coming from and where my own custom mapping went. Will investigate this more. Well that was a false alarm because I didn't rebuild my frontend and therefore it didn't pass the scopes back to Authentik when redirecting.
🎉 1
waza-ari commented 2024-03-04 17:48:09 +00:00
Author
Contributor
Copy Link

It's probably not a real problem. Maybe best to just ignore the problem and let users who are already using this (probably us two) delete the old teams manually with a delete from teams where issuer is null.

Its basically a tradeoff between a little pain for more advanced users now (with limited impact) or having extra logic in the code that'll probably sit there, do nothing in 99% of the cases and might cause questions in the future. So yeah I'd agree here, sounds sensible to me

I've noticed another odd thing in my own testing: Even though I added the vikunja_groups scope and mapping, Authentik does not return it properly. It does, however, return a groups claim in the token with all groups the user is part of. Testing with GitLab, it returns a groups_direct property in the token with all GitLab organizations I am part of. Not sure where that's coming from and where my own custom mapping went. Will investigate this more.

Sadly I've absolutely no experience with Authentic. Did you configure the mapper to add it to the AT or ID token? If you want I can play around with Authentic a bit more.

In the original PR I commented that the structure that's currently required is a bit odd and probably doesn't come out of the box for basically any IdP (although admittedly I didn't express myself very well). For Gitlab I have absolutely no idea how to achieve this, as Gitlab doesn't allow any custom claims or custom mappers according to the docs.

What about setting it to false? Where did you see the admin value being set?
(As I said, this could be a follow-up)

As far as I can tell it's currently set automatically when creating a Team in the existing model. See pkg/models/teams.go L293ff:

func (t *Team) Create(s *xorm.Session, a web.Auth) (err error) {
	doer, err := user.GetFromAuth(a)
	if err != nil {
		return err
	}

	// Check if we have a name
	if t.Name == "" {
		return ErrTeamNameCannotBeEmpty{}
	}

	t.CreatedByID = doer.ID
	t.CreatedBy = doer

	_, err = s.Insert(t)
	if err != nil {
		return
	}

	tm := TeamMember{TeamID: t.ID, Username: doer.Username, Admin: true}
	if err = tm.Create(s, doer); err != nil {
		return err
	}

	return events.Dispatch(&TeamCreatedEvent{
		Team: t,
		Doer: a,
	})
}

Assumed changing this to false would lead to teams created in the UI to not have an admin, so not sure that's straight forward to change.

> It's probably not a real problem. Maybe best to just ignore the problem and let users who are already using this (probably us two) delete the old teams manually with a `delete from teams where issuer is null`. Its basically a tradeoff between a little pain for more advanced users now (with limited impact) or having extra logic in the code that'll probably sit there, do nothing in 99% of the cases and might cause questions in the future. So yeah I'd agree here, sounds sensible to me --- > I've noticed another odd thing in my own testing: Even though I added the `vikunja_groups` scope and mapping, Authentik does not return it properly. It does, however, return a `groups` claim in the token with all groups the user is part of. Testing with GitLab, it returns a `groups_direct` property in the token with all GitLab organizations I am part of. Not sure where that's coming from and where my own custom mapping went. Will investigate this more. Sadly I've absolutely no experience with Authentic. Did you configure the mapper to add it to the AT or ID token? If you want I can play around with Authentic a bit more. In the original PR I commented that the structure that's currently required is a bit odd and probably doesn't come out of the box for basically any IdP (although admittedly I didn't express myself very well). For Gitlab I have absolutely no idea how to achieve this, as Gitlab doesn't allow any custom claims or custom mappers [according to the docs](https://docs.gitlab.com/ee/integration/openid_connect_provider.html). --- > What about setting it to false? Where did you see the admin value being set? > (As I said, this could be a follow-up) As far as I can tell it's currently set automatically when creating a Team in the existing model. See `pkg/models/teams.go` L293ff: ``` func (t *Team) Create(s *xorm.Session, a web.Auth) (err error) { doer, err := user.GetFromAuth(a) if err != nil { return err } // Check if we have a name if t.Name == "" { return ErrTeamNameCannotBeEmpty{} } t.CreatedByID = doer.ID t.CreatedBy = doer _, err = s.Insert(t) if err != nil { return } tm := TeamMember{TeamID: t.ID, Username: doer.Username, Admin: true} if err = tm.Create(s, doer); err != nil { return err } return events.Dispatch(&TeamCreatedEvent{ Team: t, Doer: a, }) } ``` Assumed changing this to `false` would lead to teams created in the UI to not have an admin, so not sure that's straight forward to change.
konrad commented 2024-03-04 17:49:02 +00:00
Owner
Copy Link

Anyway, since the sync also removes old teams when no one is part of that team any more, it seems like the problem of "old teams without an issuer remaining" is not a problem.

Anyway, since the sync also removes old teams when no one is part of that team any more, it seems like the problem of "old teams without an issuer remaining" is not a problem.
waza-ari commented 2024-03-04 17:50:36 +00:00
Author
Contributor
Copy Link

Anyway, since the sync also removes old teams when no one is part of that team any more, it seems like the problem of "old teams without an issuer remaining" is not a problem.

Now that you mention that - I do wonder if the issuer is taken into account correctly when deleting teams. Just to avoid a situation where a user logging in through provider A would cause teams created by provider B to be deleted - let me verify that real quick

> Anyway, since the sync also removes old teams when no one is part of that team any more, it seems like the problem of "old teams without an issuer remaining" is not a problem. Now that you mention that - I do wonder if the issuer is taken into account correctly when deleting teams. Just to avoid a situation where a user logging in through provider A would cause teams created by provider B to be deleted - let me verify that real quick
konrad commented 2024-03-04 17:53:49 +00:00
Owner
Copy Link

In the original PR I commented that the structure that's currently required is a bit odd and probably doesn't come out of the box for basically any IdP (although admittedly I didn't express myself very well). For Gitlab I have absolutely no idea how to achieve this, as Gitlab doesn't allow any custom claims or custom mappers according to the docs.

I think that's fine because while it requires more work from the IdP, it also allows more flexibility. For example, Admins in Authentik are part of an "Authentik Default Admins" team, which is always present in the token claim. Using a custom mapper allows me to filter that out if I don't want to create it in Vikunja.

Now that you mention that - I do wonder if the issuer is taken into account correctly when deleting teams. Just to avoid a situation where a user logging in through provider A would cause teams created by provider B to be deleted - let me verify that real quick

I think it's not. I'd rather move that into a cron job instead of doing it in the auth request, since that only makes that request longer.

As far as I can tell it's currently set automatically when creating a Team in the existing model. See pkg/models/teams.go L293ff:

Yeah the way to go here is probably to refactor that out in a new function createNewTeam or similar with an extra boolean parameter if the first user should be made admin.

> In the original PR I commented that the structure that's currently required is a bit odd and probably doesn't come out of the box for basically any IdP (although admittedly I didn't express myself very well). For Gitlab I have absolutely no idea how to achieve this, as Gitlab doesn't allow any custom claims or custom mappers according to the docs. I think that's fine because while it requires more work from the IdP, it also allows more flexibility. For example, Admins in Authentik are part of an "Authentik Default Admins" team, which is always present in the token claim. Using a custom mapper allows me to filter that out if I don't want to create it in Vikunja. > Now that you mention that - I do wonder if the issuer is taken into account correctly when deleting teams. Just to avoid a situation where a user logging in through provider A would cause teams created by provider B to be deleted - let me verify that real quick I think it's not. I'd rather move that into a cron job instead of doing it in the auth request, since that only makes that request longer. > As far as I can tell it's currently set automatically when creating a Team in the existing model. See pkg/models/teams.go L293ff: Yeah the way to go here is probably to refactor that out in a new function `createNewTeam` or similar with an extra boolean parameter if the first user should be made admin.
waza-ari commented 2024-03-04 18:13:45 +00:00
Author
Contributor
Copy Link

Now that you mention that - I do wonder if the issuer is taken into account correctly when deleting teams. Just to avoid a situation where a user logging in through provider A would cause teams created by provider B to be deleted - let me verify that real quick

I think it's not. I'd rather move that into a cron job instead of doing it in the auth request, since that only makes that request longer.

It should be fine as far as I can tell. Essentially RemoveEmptySSOTeams is only called for teams the user has actually left (that is, teams the user has been a member of). As the user can only be member of teams created by the same issuer, it implicitly makes sure nothing else is deleted.

Yeah the way to go here is probably to refactor that out in a new function createNewTeam or similar with an extra boolean parameter if the first user should be made admin.

I've taken a crack at that, however not feeling very confident. Happy to revert and move into another PR if that's no good. Tests are passing locally and it seems to work (admin if I create it myself, no admin if created through OIDC)

> > Now that you mention that - I do wonder if the issuer is taken into account correctly when deleting teams. Just to avoid a situation where a user logging in through provider A would cause teams created by provider B to be deleted - let me verify that real quick > > I think it's not. I'd rather move that into a cron job instead of doing it in the auth request, since that only makes that request longer. It should be fine as far as I can tell. Essentially `RemoveEmptySSOTeams` is only called for teams the user has actually left (that is, teams the user has been a member of). As the user can only be member of teams created by the same issuer, it implicitly makes sure nothing else is deleted. > Yeah the way to go here is probably to refactor that out in a new function `createNewTeam` or similar with an extra boolean parameter if the first user should be made admin. I've taken a crack at that, however not feeling very confident. Happy to revert and move into another PR if that's no good. Tests are passing locally and it seems to work (admin if I create it myself, no admin if created through OIDC)
waza-ari added 1 commit 2024-03-04 18:14:27 +00:00
konrad commented 2024-03-04 18:30:32 +00:00
Owner
Copy Link

I've taken a crack at that, however not feeling very confident. Happy to revert and move into another PR if that's no good. Tests are passing locally and it seems to work (admin if I create it myself, no admin if created through OIDC)

It's quite good already! Please move this to a new PR anyway since that's not really what this PR set out to solve.

> I've taken a crack at that, however not feeling very confident. Happy to revert and move into another PR if that's no good. Tests are passing locally and it seems to work (admin if I create it myself, no admin if created through OIDC) It's quite good already! Please move this to a new PR anyway since that's not really what this PR set out to solve.
konrad closed this pull request 2024-03-04 18:30:32 +00:00
konrad reopened this pull request 2024-03-04 18:30:37 +00:00
waza-ari added 1 commit 2024-03-04 19:03:07 +00:00
waza-ari force-pushed fix-team-creation from 71d729dc01 to 3e068f0142 2024-03-04 19:03:37 +00:00 Compare
konrad approved these changes 2024-03-04 20:25:38 +00:00
konrad merged commit a3154e805c into main 2024-03-04 20:26:47 +00:00
konrad deleted branch fix-team-creation 2024-03-04 20:26:47 +00:00
konrad commented 2024-03-04 20:26:53 +00:00
Owner
Copy Link

Thanks!

Thanks!
Sign in to join this conversation.
Reviewers
No reviewers
konrad
Labels
Clear labels   
dependencies

   
duplicate

This issue or pull request already exists

  
help wanted

Need some help

  
invalid

Something is wrong

  
kind/bug

Something is not working

  
kind/feature

New feature

  
needs reproduction

   
question

More information is needed

  
security

   
wontfix

This won't be fixed

No Label
dependencies
duplicate
help wanted
invalid
kind/bug
kind/feature
needs reproduction
question
security
wontfix
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: vikunja/vikunja#2152
No description provided.
Delete Branch "waza-ari/vikunja:fix-team-creation"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?