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

BED-4766: API Endpoint to List OIDC Providers #896

Merged
merged 31 commits into from
Oct 15, 2024
Merged

BED-4766: API Endpoint to List OIDC Providers #896

merged 31 commits into from
Oct 15, 2024

Conversation

iustinum
Copy link
Contributor

@iustinum iustinum commented Oct 4, 2024

Description

This pull request introduces a unified SSO endpoint that provides detailed information on both OIDC and SAML providers. The goal is to offer a consolidated and accurate list of available authentication methods, ensuring that users are directed to the appropriate provider for successful authentication.

Motivation and Context

This PR addresses the requirements outlined in ticket BED-4766.

Previously, SSO providers were listed using the GET /api/v2/saml/sso endpoint, which only supported SAML providers. With the addition of OIDC as an alternative authentication method to comply with FedRamp requirements, there is a need for a single, unified endpoint. This new endpoint will display information about both SAML and OIDC providers as a more flexible and agnostic approach. The unified endpoint will replace the existing SAML-only endpoint to ensure all authentication options are covered.

How Has This Been Tested?

Unit Tests: Via auth/sso_test to verify endpoint behavior.
Integration Tests: Via database/sso_providers_test to verify interaction with the database.

Screenshots (optional):

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

@iustinum iustinum added the api A pull request containing changes affecting the API code. label Oct 4, 2024
@iustinum iustinum self-assigned this Oct 4, 2024
@juggernot325 juggernot325 added the blocked This pull request cannot be completed and should not be merged label Oct 7, 2024
@iustinum iustinum requested a review from ddlees October 7, 2024 20:12
Copy link
Contributor

@mistahj67 mistahj67 left a comment

Choose a reason for hiding this comment

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

I think you can point the base to @mvlipka 's create PR and it might help reviewing this without his changes as well? And then we just make sure we don't merge until his is merged. Looking good so far!

cmd/api/src/api/v2/auth/sso.go Outdated Show resolved Hide resolved
ddlees
ddlees previously requested changes Oct 9, 2024
cmd/api/src/database/sso_providers.go Show resolved Hide resolved
@iustinum iustinum added the work in progress This pull request is a work in progress and should not be merged label Oct 9, 2024
@iustinum iustinum removed the blocked This pull request cannot be completed and should not be merged label Oct 11, 2024
Copy link
Contributor

@mistahj67 mistahj67 left a comment

Choose a reason for hiding this comment

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

Only blocked because the permission requirement needs to be removed. Otherwise this is looking great! I concede, the gorm joins feature is schnazzy just don't let @superlinkx know I said that out loud.

Also did you run prep-for-codereview 👀

cmd/api/src/api/registration/v2.go Outdated Show resolved Hide resolved
cmd/api/src/api/v2/auth/sso.go Show resolved Hide resolved
cmd/api/src/api/v2/auth/sso.go Show resolved Hide resolved
cmd/api/src/api/v2/auth/sso.go Outdated Show resolved Hide resolved
cmd/api/src/database/sso_providers.go Show resolved Hide resolved
cmd/api/src/api/v2/auth/sso_test.go Show resolved Hide resolved
iustinum and others added 2 commits October 11, 2024 15:16
Co-authored-by: mistahj67 <26472282+mistahj67@users.noreply.github.com>
Copy link
Contributor

@mistahj67 mistahj67 left a comment

Choose a reason for hiding this comment

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

Tested locally, everything is working solid! 🚀

cmd/api/src/api/v2/analysisrequest.go Outdated Show resolved Hide resolved
cmd/api/src/api/v2/auth/sso.go Outdated Show resolved Hide resolved
cmd/api/src/api/v2/auth/sso.go Outdated Show resolved Hide resolved
cmd/api/src/api/v2/auth/sso.go Outdated Show resolved Hide resolved
iustinum and others added 4 commits October 15, 2024 13:51
Co-authored-by: mistahj67 <26472282+mistahj67@users.noreply.github.com>
Co-authored-by: mistahj67 <26472282+mistahj67@users.noreply.github.com>
Co-authored-by: mistahj67 <26472282+mistahj67@users.noreply.github.com>
Co-authored-by: mistahj67 <26472282+mistahj67@users.noreply.github.com>
@iustinum iustinum enabled auto-merge (squash) October 15, 2024 20:54
@iustinum iustinum removed the work in progress This pull request is a work in progress and should not be merged label Oct 15, 2024
@iustinum iustinum merged commit 403c45c into main Oct 15, 2024
4 checks passed
@iustinum iustinum deleted the BED-4766-V2 branch October 15, 2024 21:07
@github-actions github-actions bot locked and limited conversation to collaborators Oct 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api A pull request containing changes affecting the API code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants