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/legacy-interopt/service-resolver #2524

Merged
merged 2 commits into from
Nov 1, 2024

Conversation

odinr
Copy link
Collaborator

@odinr odinr commented Nov 1, 2024

Why

Issue when merging services from service discovery and legacy auth container (incorrect audience/scope)

LegacyAuthContainer

Fixed LegacyAuthContainer.registerAppAsync to not create duplicate AuthApps when additional resources are added to the app.

createServiceResolver

Fixed createServiceResolver to extract app client id from each services.
Previously we assumed that all services registered to the legacy auth container would use the same scope as all other services. This is not the case, as each service can have its own scope. This change allows us to extract the client id from the service definition, which is then used to create the service resolver.

Resources are indexed by the client id, so when acquiring a resource, the legacy auth container will use the client id to generate an auth token. This token is then used to authenticate the request to the resource.

NOTE: This will and should be deprecated in the future! This "bug" was discovered while an application used a mixed of legacy and new Framework, which caused the application to fail to authenticate requests to the resource (wrong audience).

Check off the following:

  • Confirm that I checked changes to branch which I am merging into.

    • I have validated included files
    • My code does not generate new linting warnings
    • My PR is not a duplicate, check existing pr`s
  • Confirm that the I have completed the self-review checklist.

  • Confirm that my changes meet our code of conduct.

Copy link

changeset-bot bot commented Nov 1, 2024

🦋 Changeset detected

Latest commit: 0f86117

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@equinor/fusion-framework-legacy-interopt Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@odinr odinr marked this pull request as ready for review November 1, 2024 12:44
@odinr odinr requested a review from a team as a code owner November 1, 2024 12:44
@odinr odinr enabled auto-merge (squash) November 1, 2024 12:46
Copy link
Contributor

github-actions bot commented Nov 1, 2024

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 0.39% 1746 / 440987
🔵 Statements 0.39% 1746 / 440987
🔵 Functions 23.8% 219 / 920
🔵 Branches 37.34% 400 / 1071
File Coverage
File Stmts % Branch % Funcs % Lines Uncovered Lines
Changed Files
packages/react/legacy-interopt/src/LegacyAuthContainer.ts 0% 0% 0% 0% 1, 3-4, 8, 27-28, 30-33, 35-37, 39-58, 60-67, 69, 71-83, 85-87, 89-94, 96-116, 118-131, 133-136, 138, 140-143, 145-151, 153-160, 162-163, 166
packages/react/legacy-interopt/src/create-service-resolver.ts 0% 0% 0% 0% 1, 5-18, 20-23, 25-27, 29-33, 37-39, 41-50, 52-70, 72
Generated in workflow #7903 for commit 0f86117 by the Vitest Coverage Report Action

@odinr odinr merged commit 1941b76 into main Nov 1, 2024
9 of 10 checks passed
@odinr odinr deleted the fix/legacy-interopt/service-resolver branch November 1, 2024 12:48
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.

2 participants