Skip to content

Add per-domain OAuth (Google, GitHub) provider support#12702

Draft
Damans227 wants to merge 40 commits intoapache:mainfrom
Damans227:oauth-per-domain
Draft

Add per-domain OAuth (Google, GitHub) provider support#12702
Damans227 wants to merge 40 commits intoapache:mainfrom
Damans227:oauth-per-domain

Conversation

@Damans227
Copy link
Copy Markdown
Collaborator

@Damans227 Damans227 commented Feb 25, 2026

Description

Add per-domain OAuth provider support. Allows OAuth providers (Google, GitHub) to be configured at the domain level with global fallback.

Design Doc

https://cwiki.apache.org/confluence/display/CLOUDSTACK/Allow+OAuth+provider+per+domain

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • Build/CI
  • Test (unit or integration test code)

Screenshots and recording

Screenshot 2026-03-16 at 10 37 06 AM Screenshot 2026-03-16 at 10 37 51 AM
Screen.Recording.2026-03-16.at.10.34.53.AM.mp4

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

How Has This Been Tested?

Manual testing with GitHub OAuth provider configured at domain level and global level, verifying domain-specific lookup with global fallback.

Daman Arora and others added 19 commits December 18, 2025 11:23
@Damans227
Copy link
Copy Markdown
Collaborator Author

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

@Damans227 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link
Copy Markdown

Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 16931

@Damans227
Copy link
Copy Markdown
Collaborator Author

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

@Damans227 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link
Copy Markdown

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17098

* Verifies the code provided by provider and fetches email for a specific domain
* @return returns email
*/
String verifyCodeAndFetchEmail(String secretCode, Long domainId);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
String verifyCodeAndFetchEmail(String secretCode, Long domainId);
String verifySecretCodeAndFetchEmail(String secretCode, Long domainId);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@sureshanaparti The domain-aware verifyCodeAndFetchEmail(String secretCode, Long domainId) is an overload of the pre-existing verifyCodeAndFetchEmail(String secretCode). If we rename only the new overload to verifySecretCodeAndFetchEmail, the two methods would have inconsistent names. Should we rename both for consistency?


@Override
public Long resolveDomainId(Map<String, Object[]> params) {
final String[] domainIdArray = (String[])params.get(ApiConstants.DOMAIN_ID);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

API cmd uses domainId - ApiConstants.DOMAIN__ID (camelcase param - not deprecated yet)

@Parameter(name = ApiConstants.DOMAIN, type = CommandType.STRING, description = "Path of the domain that the user belongs to. Example: domain=/com/cloud/internal. If no domain is passed in, the ROOT (/) domain is assumed.")
private String domain;
@Parameter(name = ApiConstants.DOMAIN__ID, type = CommandType.LONG, description = "The id of the domain that the user belongs to. If both domain and domainId are passed in, \"domainId\" parameter takes precedence.")
private Long domainId;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@sureshanaparti Just looked into it, both getDomainIdFromParams and resolveDomainId look up ApiConstants.DOMAIN_ID ("domainid"). The API servlet lowercases param names, so DOMAIN__ID ("domainId") resolves correctly.

if (Objects.nonNull(domain)) {
return domain.getId();
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

check if you can use the below methods, and move get domain by path to a method (or can improve any existing method)

public String getDomainId(Map<String, Object[]> params) {

public Long fetchDomainId(final String domainUUID) {

Copy link
Copy Markdown
Collaborator Author

@Damans227 Damans227 Mar 27, 2026

Choose a reason for hiding this comment

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

@sureshanaparti Looked into the ApiServer methods. fetchDomainId only resolves by UUID, while resolveDomainId also handles domain path resolution. The closest existing combination would be fetchDomainId + DomainService.findDomainByIdOrPath, which is what OauthLoginAPIAuthenticatorCmd already uses. I could refactor resolveDomainId to delegate to those, but since OAuth2AuthManagerImpl doesn't have access to ApiServer, it would mean injecting DomainService instead of DomainDao. Would you prefer that approach, or is the current implementation acceptable?

@Damans227
Copy link
Copy Markdown
Collaborator Author

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

@Damans227 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link
Copy Markdown

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17268

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants