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

add simple provider #97

Merged
merged 3 commits into from
Nov 28, 2023
Merged

add simple provider #97

merged 3 commits into from
Nov 28, 2023

Conversation

anvddriesch
Copy link
Contributor

Towards giantswarm/roadmap#2988

This adds the simple provider to dex operator.
The provider works by simply copying existing connector data to dex-apps across an MC and injecting the callback URIs.
It has the following benefits

  • dex-operator does not need access to the idp
  • dex-operator does not need to support the connector type
  • the same access that is available on the MC can automatically be provided on each WC
  • formatting of the connector is validated

@anvddriesch anvddriesch self-assigned this Nov 27, 2023
@anvddriesch anvddriesch requested a review from a team November 27, 2023 11:57

func usesRedirectURI(connectorType string) bool {
switch connectorType {
case "ldap", "authproxy", "atlassian-crowd", "keystone":
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These types do not use callback so it would cause an error to inject redirectURI.

@@ -197,6 +198,16 @@ func getProvidersFromConfig(credentials Config, include string, log logr.Logger)
return providers, nil
}

func includeProvider(include string, provider string) bool {
if provider == simpleprovider.ProviderName {
return false
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The simple type does not allow access to the idp and therefore can not create a new credential. It needs to be set up manually.
When updating credentials however, the old one will be kept.

@kopiczko
Copy link

I'm fine with the code, but it would be good to add some documentation.

You forgot to add "if one credential leaks then all clusters are compromised" to the list of benefits :P

@anvddriesch
Copy link
Contributor Author

@kopiczko I added some documentation with stronger mention of the implications.

@anvddriesch anvddriesch merged commit e36f088 into main Nov 28, 2023
2 checks passed
@anvddriesch anvddriesch deleted the default-provider branch November 28, 2023 11:49
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

Successfully merging this pull request may close these issues.

2 participants