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

fix(OSP Invite): Handle theme for shared realm #852

Conversation

dhiren-singh-007
Copy link
Contributor

@dhiren-singh-007 dhiren-singh-007 commented Jul 19, 2024

Description

Modified the GetCompanyAndApplicationDetailsForApprovalAsync method to get only shared idp alias

fix #848 close #848

Why

Earlier if we use Managed idp(eg using Azure AD) then application activation was failing because code was trying to set theme on shared realm, which don't exist.

Issue

Issue 848

Checklist

Please delete options that are not relevant.

  • I have followed the contributing guidelines
  • I have performed a self-review of my own code
  • I have successfully tested my changes locally
  • I have checked that new and existing tests pass locally with my changes

Copy link
Contributor

@evegufy evegufy left a comment

Choose a reason for hiding this comment

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

v2.1.0 is closed (for everything more than a typo ;)), please change to main

@evegufy evegufy changed the base branch from release/v2.1.0-RC2 to main July 29, 2024 11:46
@dhiren-singh-007 dhiren-singh-007 force-pushed the bugfix/848-osp-invite-shared-theme branch from 31c98e5 to d6a0bdb Compare August 1, 2024 09:11
@dhiren-singh-007 dhiren-singh-007 requested review from Phil91 and removed request for Phil91 and evegufy August 2, 2024 08:34
@typecastcloud
Copy link
Contributor

@Phil91 @ntruchsess @MaximilianHauer

This is also a breaking issue for OSP. Only keycloak as idp worked for us.

We tested with Azure Entra and it tries to set theme, which it is not able to do.

Was this setting the theme of the OSP keycloak client before? Might also not be a good idea to overwrite OSP themes.

@dhiren-singh-007 dhiren-singh-007 requested review from Phil91 and evegufy and removed request for evegufy August 5, 2024 07:51
@evegufy
Copy link
Contributor

evegufy commented Aug 5, 2024

Hi @dhiren-singh-007 thank you for the contribution!
@ntruchsess could you please review as well?

Phil91
Phil91 previously approved these changes Aug 6, 2024
@evegufy evegufy changed the base branch from main to release/v2.2.0-RC2 August 8, 2024 16:07
@evegufy evegufy dismissed their stale review August 12, 2024 16:12

dismiss due to Phil and Norbert reviewing

@dhiren-singh-007 dhiren-singh-007 force-pushed the bugfix/848-osp-invite-shared-theme branch from 65a6b62 to 078d548 Compare August 13, 2024 15:17
@dhiren-singh-007
Copy link
Contributor Author

Thanks @ntruchsess for the suggestion and giving more insights . I reverted all the commits and implemented the suggestion . Please re-review .

Copy link

sonarcloud bot commented Aug 13, 2024

@evegufy evegufy merged commit be8f1d7 into eclipse-tractusx:release/v2.2.0-RC2 Aug 14, 2024
11 checks passed
@dhiren-singh-007 dhiren-singh-007 deleted the bugfix/848-osp-invite-shared-theme branch August 14, 2024 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: USER READY
Development

Successfully merging this pull request may close these issues.

OSP Invite - For managed IDP it got failed with error :clientId saidp** not found on shared idp
5 participants