Skip to content
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

feature: Load oauth_connect.json and oauth_login.json from disk if the files exist #942

Closed
unreality opened this issue Nov 24, 2022 · 8 comments
Assignees

Comments

@unreality
Copy link

In order to easily add new OAuth providers without needing to edit the source, it would be useful if windmill checked if oauth_login.json and oauth_connect.json were present in the local directory first, and use the included src version if they do not exist.

Certain OAuth provides (such as AzureAD) require the authorisation and token endpoints to be specific for the tenant, so a generic URL will not be suitable for them.

This seems like the simplest way currently of enabling adding new OAuth endpoints at the moment since the checks can be added to

let connect_configs = serde_json::from_str::<HashMap<String, OAuthConfig>>(include_str!(
where the check for oauth.json already exists

@rubenfiszel
Copy link
Contributor

That's one way to do it but I'd rather make the url templatable with https://{foo}.bar/token, and add a replace_all: {"foo": "me"} objet field to oauth.json that would apply replace_all to those urls. This way the oauth_login and oauth_connect will be enriched overtime of all those providers.

I am also considering storing those oauth endpoints in the database such that each workspace can specialize its oauth.

@unreality
Copy link
Author

unreality commented Nov 24, 2022

Another option might be to support openid-configuration endpoints within oauth.json which contain the required endpoints, which would let users specify any OIDC provider.

For example:

@rubenfiszel
Copy link
Contributor

rubenfiszel commented Nov 24, 2022

That would be very powerful indeed. I'm thinking it could be an openid.json/txt file or a table (workspace_id, endpoint) that contains all the .well-known endpoints as a list. I will look into it, thanks for the pointers!

@rubenfiszel
Copy link
Contributor

@unreality Added the ability to add your own custom idP provider in oauth.json as documented in: https://github.com/windmill-labs/windmill#oauth-for-self-hosting-very-optional

Lmk if that is sufficient for your needs

@rubenfiszel
Copy link
Contributor

In ae635a4, the custom IdP is also now listed in the login page

@unreality
Copy link
Author

Hi @rubenfiszel, i tried out the latest main build, and it looks like it successfully redirects, shows the additional login option when a login_config is defined, however when it gets to the login_callback it produces an error:

2022-11-25T00:47:26.197999Z ERROR request: windmill_common::error: error="Bad request: client name not recognized" method=POST uri=/api/oauth/login_callback/azuread

When my client name is azuread

I had a quick look through oauth2.rs but couldn't figure out where it was being generated, so im afraid cant give any pointers for what might be causing this..

@rubenfiszel
Copy link
Contributor

@unreality on latest commit, you would just need to provide the proper userinfo_url as client config and it should work. Sorry for the trouble.
For instance for google it is:

  "google": {
    "auth_url": "https://accounts.google.com/o/oauth2/v2/auth",
    "token_url": "https://oauth2.googleapis.com/token",
    "userinfo_url": "https://www.googleapis.com/oauth2/v1/userinfo?alt=json",
    "scopes": ["https://www.googleapis.com/auth/userinfo.email"]
  }

@unreality
Copy link
Author

Thanks @rubenfiszel that has solved the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants