-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add per-domain OAuth (Google, GitHub) provider support #12702
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
Draft
Damans227
wants to merge
59
commits into
apache:main
Choose a base branch
from
Damans227:oauth-per-domain
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
59 commits
Select commit
Hold shift + click to select a range
dfc45c0
Add domain_id to oauth_provider table and VO
fcb4778
Add domain-aware methods to OauthProviderDao
85c64e5
Add domainId parameter to OAuth provider API commands and response
c46fb98
Add domain support to OAuth2AuthManager
4ef100c
Add domain-aware OAuth verification
8e48938
Add domain support to ListOAuthProvidersCmd and update related tests
9ab26db
fix domain path issue
3e7fa92
Add domainId support to OAuth provider
4a6b28e
Return domain name and UUID in OAuth provider API responses using Api…
Damans227 3ab3ca9
Refactor domain ID resolution in VerifyOAuthCodeAndGetUserCmd to impr…
Damans227 bb6f137
Enhance OAuth2 plugin support for domain-level configuration and auth…
Damans227 764495b
Update OAuth2 tests and VerifyOAuthCodeAndGetUserCmdTest
Damans227 0ec6bcc
Add method to find OAuth provider by domain with global fallback
Damans227 9609ef0
Update OAuth provider configuration to use 'domain' instead of 'domai…
Damans227 65ccef1
Refactor OAuth provider methods to support domain-level queries and e…
Damans227 e3da679
Add caching for access token retrieval in GithubOAuth2Provider
Damans227 0e32a60
Refactor access token checks in GithubOAuth2Provider to use StringUti…
Damans227 23c4132
Refactor null checks to use utility for improved readability and cons…
Damans227 1a23f3a
Update OAuth2UserAuthenticatorTest to include domainId in user verifi…
Damans227 b8cf181
Merge branch 'main' into oauth-per-domain
Damans227 6d73964
Remove unnecessary blank line and unused imports in OAuth provider co…
Damans227 a428d1a
Refactor and cleanup
Damans227 b367434
Remove unnecessary blank lines
Damans227 42a8651
Enhance RegisterOAuthProviderCmdTest with additional provider mock data
Damans227 7a55ecb
Remove startup gate from OAuth plugin initialization to support dynam…
Damans227 57c837c
Add strictScope to ConfigKey to disable global fallback for domain-sc…
Damans227 62d4fc3
Add domain-scoped provider filtering to listOauthProvider and central…
Damans227 0bab9f6
Add External OAuth tab with domain-scoped provider selection to login…
Damans227 94c9e4b
code cleanup
Damans227 1a45270
test fix
Damans227 24ca473
Handle login page provider visibility
Damans227 df0ff42
UI cleanup
Damans227 0f06f99
UI Cleanup
Damans227 868b0f4
Keep text color consistent
Damans227 b4b7420
add unit tests
Damans227 dc30f14
Add Multiple-domain OAuth tests
Damans227 2f9001b
Refactor as per PR comments
Damans227 2185237
Use idempotent DDL helpers for oauth_provider schema migration
Damans227 2fe8d53
Use global config check for global providers and extract oauthEnabled…
Damans227 3b3288c
Make strictScope return null when id is null
Damans227 ac4a622
Rename verification methods to use 'verifySecretCodeAndFetchEmail' fo…
Damans227 db7f290
Refactor domain handling in OAuth2AuthManagerImpl to use DomainServic…
Damans227 1e99cce
Enhance domain ID descriptions in OAuth command classes for clarity
Damans227 974aadc
Add domain path handling to OAuth provider commands and improve descr…
Damans227 df8c869
Update domain path description in VerifyOAuthCodeAndGetUserCmd to cla…
Damans227 911c794
Replace remove method with expunge in deleteOauthProvider and add cor…
Damans227 23796d0
Add external login label to Login.vue and update i18n locale handling
Damans227 bd74b27
Fix stale value issue in updateConfiguration response handling in Con…
Damans227 4c38514
Enhance OAuth login error handling and add unit test for missing para…
Damans227 96d4c69
Add validation to reject enabling OAuth provider when plugin is disab…
Damans227 16a15d3
Add domain reassignment support to UpdateOAuthProviderCmd and enhance…
Damans227 290d80a
Add domain ID to OAuth provider arguments in config
Damans227 9ae9983
Fix condition for OAuth verification URL handling in router
Damans227 b6d9aa5
Add domain path to OauthProviderResponse and update UI config to disp…
Damans227 4e3c8e9
Update config to remove 'secretkey' from columns and details
Damans227 3b2ef3f
Add secretkey to details in config and display in DetailsTab
Damans227 b056914
Implement normalization of ROOT domain to null for global OAuth provi…
Damans227 01fb9a8
Refactor OAuth plugin domain scope handling to use a centralized meth…
Damans227 e4c3b3d
Add strict scope handling to ConfigKey and update OAuth2AuthManager u…
Damans227 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,10 +18,13 @@ | |
| // | ||
| package org.apache.cloudstack.oauth2; | ||
|
|
||
| import com.cloud.domain.Domain; | ||
| import com.cloud.user.DomainService; | ||
| import com.cloud.user.dao.UserDao; | ||
| import com.cloud.utils.component.Manager; | ||
| import com.cloud.utils.component.ManagerBase; | ||
| import com.cloud.utils.exception.CloudRuntimeException; | ||
| import org.apache.cloudstack.api.ApiConstants; | ||
| import org.apache.cloudstack.auth.UserOAuth2Authenticator; | ||
| import org.apache.cloudstack.framework.config.ConfigKey; | ||
| import org.apache.cloudstack.framework.config.Configurable; | ||
|
|
@@ -35,12 +38,15 @@ | |
| import org.apache.cloudstack.oauth2.vo.OauthProviderVO; | ||
| import org.apache.commons.lang3.StringUtils; | ||
|
|
||
| import org.apache.commons.lang3.ArrayUtils; | ||
|
|
||
| import javax.inject.Inject; | ||
| import java.util.ArrayList; | ||
| import java.util.Collections; | ||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
|
|
||
| public class OAuth2AuthManagerImpl extends ManagerBase implements OAuth2AuthManager, Manager, Configurable { | ||
| @Inject | ||
|
|
@@ -49,6 +55,9 @@ public class OAuth2AuthManagerImpl extends ManagerBase implements OAuth2AuthMana | |
| @Inject | ||
| protected OauthProviderDao _oauthProviderDao; | ||
|
|
||
| @Inject | ||
| private DomainService _domainService; | ||
|
|
||
| protected static Map<String, UserOAuth2Authenticator> userOAuth2AuthenticationProvidersMap = new HashMap<>(); | ||
|
|
||
| private List<UserOAuth2Authenticator> userOAuth2AuthenticationProviders; | ||
|
|
@@ -64,17 +73,13 @@ public List<Class<?>> getAuthCommands() { | |
|
|
||
| @Override | ||
| public boolean start() { | ||
| if (isOAuthPluginEnabled()) { | ||
| logger.info("OAUTH plugin loaded"); | ||
| initializeUserOAuth2AuthenticationProvidersMap(); | ||
| } else { | ||
| logger.info("OAUTH plugin not enabled so not loading"); | ||
| } | ||
| initializeUserOAuth2AuthenticationProvidersMap(); | ||
| logger.info("OAUTH plugin loaded"); | ||
| return true; | ||
| } | ||
|
|
||
| protected boolean isOAuthPluginEnabled() { | ||
| return OAuth2IsPluginEnabled.value(); | ||
| protected boolean isOAuthPluginEnabled(Long domainId) { | ||
| return OAuth2AuthManager.isPluginEnabledForDomain(domainId); | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -125,9 +130,9 @@ protected void initializeUserOAuth2AuthenticationProvidersMap() { | |
| } | ||
|
|
||
| @Override | ||
| public String verifyCodeAndFetchEmail(String code, String provider) { | ||
| public String verifySecretCodeAndFetchEmail(String code, String provider, Long domainId) { | ||
| UserOAuth2Authenticator authenticator = getUserOAuth2AuthenticationProvider(provider); | ||
| String email = authenticator.verifyCodeAndFetchEmail(code); | ||
| String email = authenticator.verifySecretCodeAndFetchEmail(code, domainId); | ||
|
|
||
| return email; | ||
| } | ||
|
|
@@ -139,25 +144,36 @@ public OauthProviderVO registerOauthProvider(RegisterOAuthProviderCmd cmd) { | |
| String clientId = StringUtils.trim(cmd.getClientId()); | ||
| String redirectUri = StringUtils.trim(cmd.getRedirectUri()); | ||
| String secretKey = StringUtils.trim(cmd.getSecretKey()); | ||
| Long domainId = normalizeGlobalScope(resolveDomainIdFromIdOrPath(cmd.getDomainId(), cmd.getDomainPath())); | ||
|
|
||
| if (!isOAuthPluginEnabled()) { | ||
| if (!isOAuthPluginEnabled(domainId)) { | ||
| throw new CloudRuntimeException("OAuth is not enabled, please enable to register"); | ||
| } | ||
| OauthProviderVO providerVO = _oauthProviderDao.findByProvider(provider); | ||
|
|
||
| // Check for existing provider with same name and domain | ||
| OauthProviderVO providerVO = _oauthProviderDao.findByProviderAndDomain(provider, domainId); | ||
| if (providerVO != null) { | ||
| throw new CloudRuntimeException(String.format("Provider with the name %s is already registered", provider)); | ||
| if (domainId == null) { | ||
| throw new CloudRuntimeException(String.format("Global provider with the name %s is already registered", provider)); | ||
| } else { | ||
| throw new CloudRuntimeException(String.format("Provider with the name %s is already registered for domain %d", provider, domainId)); | ||
| } | ||
| } | ||
|
|
||
| return saveOauthProvider(provider, description, clientId, secretKey, redirectUri); | ||
| return saveOauthProvider(provider, description, clientId, secretKey, redirectUri, domainId); | ||
| } | ||
|
|
||
| @Override | ||
| public List<OauthProviderVO> listOauthProviders(String provider, String uuid) { | ||
| public List<OauthProviderVO> listOauthProviders(String provider, String uuid, Long domainId) { | ||
| List<OauthProviderVO> providers; | ||
| if (uuid != null) { | ||
| providers = Collections.singletonList(_oauthProviderDao.findByUuid(uuid)); | ||
| } else if (StringUtils.isNotBlank(provider) && domainId != null) { | ||
| providers = Collections.singletonList(_oauthProviderDao.findByProviderAndDomain(provider, domainId)); | ||
| } else if (StringUtils.isNotBlank(provider)) { | ||
| providers = Collections.singletonList(_oauthProviderDao.findByProvider(provider)); | ||
| providers = Collections.singletonList(_oauthProviderDao.findByProviderAndDomain(provider, null)); | ||
| } else if (domainId != null) { | ||
| providers = _oauthProviderDao.listByDomainIncludingGlobal(domainId); | ||
| } else { | ||
| providers = _oauthProviderDao.listAll(); | ||
| } | ||
|
|
@@ -178,6 +194,30 @@ public OauthProviderVO updateOauthProvider(UpdateOAuthProviderCmd cmd) { | |
| throw new CloudRuntimeException("Provider with the given id is not there"); | ||
| } | ||
|
|
||
| Long targetDomainId = providerVO.getDomainId(); | ||
| if (cmd.getDomainId() != null || StringUtils.isNotEmpty(cmd.getDomainPath())) { | ||
| Long resolved = resolveDomainIdFromIdOrPath(cmd.getDomainId(), cmd.getDomainPath()); | ||
| if (resolved == null) { | ||
| throw new CloudRuntimeException("Unable to resolve the supplied domain. Provide a valid domain id or path."); | ||
| } | ||
| resolved = normalizeGlobalScope(resolved); | ||
| if (!Objects.equals(resolved, providerVO.getDomainId())) { | ||
| OauthProviderVO existing = _oauthProviderDao.findByProviderAndDomain(providerVO.getProvider(), resolved); | ||
| if (existing != null) { | ||
| throw new CloudRuntimeException(String.format( | ||
| "Provider with the name %s is already registered for domain %s", providerVO.getProvider(), | ||
| resolved == null ? "ROOT (global)" : resolved)); | ||
| } | ||
| } | ||
| targetDomainId = resolved; | ||
| } | ||
|
|
||
| if (Boolean.TRUE.equals(enabled) && !isOAuthPluginEnabled(targetDomainId)) { | ||
| throw new CloudRuntimeException(String.format( | ||
| "OAuth plugin is not enabled %s. Enable oauth2.enabled at that scope before enabling this provider.", | ||
| targetDomainId == null ? "globally" : "for this domain")); | ||
| } | ||
|
|
||
| if (StringUtils.isNotEmpty(description)) { | ||
| providerVO.setDescription(description); | ||
| } | ||
|
|
@@ -193,20 +233,22 @@ public OauthProviderVO updateOauthProvider(UpdateOAuthProviderCmd cmd) { | |
| if (enabled != null) { | ||
| providerVO.setEnabled(enabled); | ||
| } | ||
| providerVO.setDomainId(targetDomainId); | ||
|
|
||
| _oauthProviderDao.update(id, providerVO); | ||
|
|
||
| return _oauthProviderDao.findById(id); | ||
| } | ||
|
|
||
| private OauthProviderVO saveOauthProvider(String provider, String description, String clientId, String secretKey, String redirectUri) { | ||
| private OauthProviderVO saveOauthProvider(String provider, String description, String clientId, String secretKey, String redirectUri, Long domainId) { | ||
| final OauthProviderVO oauthProviderVO = new OauthProviderVO(); | ||
|
|
||
| oauthProviderVO.setProvider(provider); | ||
| oauthProviderVO.setDescription(description); | ||
| oauthProviderVO.setClientId(clientId); | ||
| oauthProviderVO.setSecretKey(secretKey); | ||
| oauthProviderVO.setRedirectUri(redirectUri); | ||
| oauthProviderVO.setDomainId(domainId); | ||
| oauthProviderVO.setEnabled(true); | ||
|
|
||
| _oauthProviderDao.persist(oauthProviderVO); | ||
|
|
@@ -216,7 +258,66 @@ private OauthProviderVO saveOauthProvider(String provider, String description, S | |
|
|
||
| @Override | ||
| public boolean deleteOauthProvider(Long id) { | ||
| return _oauthProviderDao.remove(id); | ||
| return _oauthProviderDao.expunge(id); | ||
| } | ||
|
|
||
| @Override | ||
| public Long resolveDomainId(Map<String, Object[]> params) { | ||
| final String[] domainIdArray = (String[])params.get(ApiConstants.DOMAIN_ID); | ||
| if (ArrayUtils.isNotEmpty(domainIdArray)) { | ||
| String domainUuid = domainIdArray[0]; | ||
| if (GLOBAL_DOMAIN_FILTER.equals(domainUuid)) { | ||
| return GLOBAL_DOMAIN_ID; | ||
| } | ||
| Domain domain = _domainService.getDomain(domainUuid); | ||
| if (Objects.nonNull(domain)) { | ||
| return domain.getId(); | ||
| } | ||
| } | ||
|
Damans227 marked this conversation as resolved.
|
||
| final String[] domainArray = (String[])params.get(ApiConstants.DOMAIN); | ||
| if (ArrayUtils.isNotEmpty(domainArray)) { | ||
| String path = normalizeDomainPath(domainArray[0]); | ||
| if (StringUtils.isNotEmpty(path)) { | ||
| Domain domain = _domainService.findDomainByIdOrPath(null, path); | ||
| if (Objects.nonNull(domain)) { | ||
| return domain.getId(); | ||
| } | ||
| } | ||
| } | ||
| return null; | ||
| } | ||
|
Comment on lines
+265
to
+288
|
||
|
|
||
| protected Long resolveDomainIdFromIdOrPath(Long domainId, String domainPath) { | ||
| if (domainId != null) { | ||
| return domainId; | ||
| } | ||
| String path = normalizeDomainPath(domainPath); | ||
| if (StringUtils.isNotEmpty(path)) { | ||
| Domain domain = _domainService.findDomainByIdOrPath(null, path); | ||
| if (Objects.nonNull(domain)) { | ||
| return domain.getId(); | ||
| } | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| // The ROOT domain is the top of the tree, so a provider scoped to it is equivalent | ||
| // to a global provider; treat it as global so the global oauth2.enabled config applies. | ||
| protected Long normalizeGlobalScope(Long domainId) { | ||
| return (domainId != null && Domain.ROOT_DOMAIN == domainId) ? null : domainId; | ||
| } | ||
|
|
||
| protected String normalizeDomainPath(String path) { | ||
| if (StringUtils.isEmpty(path)) { | ||
| return null; | ||
| } | ||
| if (!path.startsWith("/")) { | ||
| path = "/" + path; | ||
| } | ||
| if (!path.endsWith("/")) { | ||
| path += "/"; | ||
| } | ||
| return path; | ||
| } | ||
|
|
||
| @Override | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.