Damans227 commented on code in PR #12702:
URL: https://github.com/apache/cloudstack/pull/12702#discussion_r3002011187
##########
plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/OAuth2AuthManagerImpl.java:
##########
@@ -219,6 +236,38 @@ public boolean deleteOauthProvider(Long id) {
return _oauthProviderDao.remove(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;
+ }
+ DomainVO domain = _domainDao.findByUuid(domainUuid);
+ if (Objects.nonNull(domain)) {
+ return domain.getId();
+ }
+ }
Review Comment:
@sureshanaparti Looked into the `ApiServer` methods.
[`fetchDomainId`](https://github.com/apache/cloudstack/blob/1bff543e58e626a7bcdea265284c8f3bcf57c7b0/server/src/main/java/com/cloud/api/ApiServer.java#L1163)
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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
