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

Modify the application patch and get requests to skip the organization role association #604

Merged
merged 2 commits into from
Apr 19, 2024

Conversation

DMHP
Copy link
Contributor

@DMHP DMHP commented Apr 8, 2024

Modify the application patch and get requests to skip the organization role association

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Hasanthi Dissanayake seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Comment on lines 856 to 860
if (associatedRoles != null) {
allowedAudience = associatedRoles.getAllowedAudience();
List<Role> listRole = associatedRoles.getRoles();
if (AssociatedRolesConfig.AllowedAudienceEnum.ORGANIZATION.equals(allowedAudience)
&& !listRole.isEmpty()) {
throw buildClientError(ErrorMessage.INVALID_ROLE_ASSOCIATION_FOR_ORGANIZATION_AUDIENCE);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we make allowedAudience is the only editable param of AssociatedRolesConfig object?
associatedRoles will be only a readonly attribute
for applicationAudience apps -> return the created App audienced roles, association will be built when creating the app audienced roles
for organizationAudiece apps -> return nothing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For organizationAudiece apps, we need to return all the organization roles available in the organization regardless of the audience.

@jenkins-is-staging
Copy link

PR builder started
Link: https://github.com/wso2/product-is/actions/runs/8734662401

@jenkins-is-staging
Copy link

PR builder completed
Link: https://github.com/wso2/product-is/actions/runs/8734662401
Status: failure

@jenkins-is-staging
Copy link

PR builder started
Link: https://github.com/wso2/product-is/actions/runs/8750673869

@jenkins-is-staging
Copy link

PR builder completed
Link: https://github.com/wso2/product-is/actions/runs/8750673869
Status: failure

@DMHP DMHP merged commit 3384206 into wso2:master Apr 19, 2024
3 of 4 checks passed
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.

5 participants