Skip to content

Add operations to create and delete OIDC provider configs. #400

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

Merged

Conversation

micahstairs
Copy link
Contributor

This adds an operation to create OIDC provider configs (can be done using either the tenant-aware or standard Firebase client), as well as an operation to delete provider configs.

I spent some time trying to see if we could make the create operation generic so that it could support both OIDC and SAML provider configs, however, I'm concerned that such a solution would add complexity to the external API, rather than making it simpler. I think it would also introduce some sort of instanceof check internally.

This work is part of adding multi-tenancy support (see issue #332).

Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

Looks pretty good. Just added a suggestion on using generics to improve the API surface a bit. See if that makes sense.

@hiranya911 hiranya911 assigned micahstairs and unassigned hiranya911 Apr 28, 2020
Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

See responses to the issues raised.

@micahstairs micahstairs changed the base branch from micahstairs-oidc-changes to idp-config April 29, 2020 14:40
@micahstairs
Copy link
Contributor Author

I've merged the other PR into this and changed the base branch to idp-config.

@micahstairs micahstairs requested a review from hiranya911 April 29, 2020 14:44
Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

One suggestion on the base URL.

Copy link
Member

@lahirumaramba lahirumaramba left a comment

Choose a reason for hiding this comment

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

LGTM

@lahirumaramba lahirumaramba removed their assignment Apr 29, 2020
Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

Thanks. LGTM

@hiranya911 hiranya911 assigned micahstairs and unassigned hiranya911 Apr 29, 2020
@micahstairs micahstairs merged commit 0c76cc6 into idp-config Apr 29, 2020
@micahstairs micahstairs deleted the micahstairs-create-and-delete-provider-configs branch April 29, 2020 23:10
micahstairs added a commit that referenced this pull request Jun 12, 2020
This adds an operation to create OIDC provider configs, as well as an operation to delete provider configs. These operations can be performed using either the tenant-aware or standard Firebase client.

This work is part of adding multi-tenancy support (see issue #332).
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.

3 participants