-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Add api support for external authentication management #34234
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add api support for external authentication management #34234
Conversation
…thub.com:uvulpos/gitea into feat/add-oauth-management-to-api-for-iac-tooling
m.Patch("/{id}", bind(api.EditAuthOauth2Option{}), admin.EditOauthAuth) | ||
m.Delete("/{id}", admin.DeleteOauthAuth) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parameter via url or url param?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless someone else has different opinion I think it should stay like this.
What about getting the current configuration for an ID?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But what's the current configuration? We can have n login sources configured and you can access it via pat so no auth source is getting used at all
Ok, I'll add this feature to get a config by ID
routers/api/v1/admin/auth_oauth.go
Outdated
|
||
listOptions := utils.GetListOptions(ctx) | ||
|
||
authSources, maxResults, err := db.FindAndCount[auth_model.Source](ctx, auth_model.FindSourcesOptions{}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filter and return just type OAuth
[Help appreciated]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure search is needed? Second opinion would be nice this though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The url explicit says oauth, so I'm not sure if I want to receive LDAP connections
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To filter for oauth only you should add LoginType: auth_model.OAuth2
to the struct.
But I was wondering why oauth search specifically instead of more generic one? The struct doesn't really let you filter by much more too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't tested the changes yet or looked too deep into it, at a glance it looks like it's a good starting point.
In case you haven't yet seen it: https://github.com/go-gitea/gitea/blob/main/cmd/admin_auth_oauth.go covers cli endpoint for adding oauth, which you might find useful.
Few minor points:
- copyright date in new files should be from this year and I don't think gogs attribution here is needed(?)
- comments in
services/convert/auth_oauth.go
don't match the functions
modules/structs/auth_oauth2.go
Outdated
Type int `json:"type"` | ||
TypeName string `json:"type_name"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why end user would be interested in numeric type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was not sure, but I think it's more unique than the name for machine readability. But... I don't know. Opinions are welcome :)
m.Patch("/{id}", bind(api.EditAuthOauth2Option{}), admin.EditOauthAuth) | ||
m.Delete("/{id}", admin.DeleteOauthAuth) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless someone else has different opinion I think it should stay like this.
What about getting the current configuration for an ID?
routers/api/v1/admin/auth_oauth.go
Outdated
|
||
listOptions := utils.GetListOptions(ctx) | ||
|
||
authSources, maxResults, err := db.FindAndCount[auth_model.Source](ctx, auth_model.FindSourcesOptions{}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure search is needed? Second opinion would be nice this though.
…uth authentication
…thub.com:uvulpos/gitea into feat/add-oauth-management-to-api-for-iac-tooling
…thub.com:uvulpos/gitea into feat/add-oauth-management-to-api-for-iac-tooling
…ourceOption usage
Summary:
As a DevOps Engineer, I want an automatic infrastructure setup with terraform and therefor have to enhance the gitea external api.
API:
Terraform (later):
Todos:
Other:
This is my first contribution to gitea so any help is appreciated