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

feat(auth)!: move consent interaction to different port #2665

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

golobitch
Copy link
Collaborator

@golobitch golobitch commented Apr 16, 2024

Changes proposed in this pull request

  • Check issue

Context

fixes #2649

Checklist

  • Related issues linked using fixes #number
  • Tests added/updated
  • Documentation added
  • Make sure that all checks pass
  • Bruno collection updated

@golobitch golobitch self-assigned this Apr 16, 2024
@golobitch golobitch linked an issue Apr 16, 2024 that may be closed by this pull request
1 task
@github-actions github-actions bot added type: source Changes business logic pkg: auth Changes in the GNAP auth package. pkg: mock-ase labels Apr 16, 2024
Copy link

netlify bot commented Apr 16, 2024

Deploy Preview for brilliant-pasca-3e80ec canceled.

Name Link
🔨 Latest commit 4135dfa
🔍 Latest deploy log https://app.netlify.com/sites/brilliant-pasca-3e80ec/deploys/6654d81f8fbae40009326d8f

@golobitch golobitch force-pushed the 2649-auth-move-interaction-routes-to-a-different-port-1 branch from a239a55 to 55effbf Compare April 16, 2024 22:27
@sabineschaller sabineschaller added the breaking Issue/PR that introduces breaking changes label Apr 17, 2024
@golobitch golobitch force-pushed the 2649-auth-move-interaction-routes-to-a-different-port-1 branch from 55effbf to 4a82474 Compare May 1, 2024 15:55
@github-actions github-actions bot added type: tests Testing related pkg: documentation Changes in the documentation package. type: documentation Improvements or additions to documentation labels May 1, 2024
@golobitch golobitch marked this pull request as ready for review May 2, 2024 22:43
packages/auth/src/app.ts Outdated Show resolved Hide resolved
packages/auth/src/app.ts Outdated Show resolved Hide resolved
test/integration/lib/test-actions/index.ts Outdated Show resolved Hide resolved
@golobitch golobitch requested a review from mkurapov May 10, 2024 20:10
@golobitch golobitch force-pushed the 2649-auth-move-interaction-routes-to-a-different-port-1 branch from 09f76e1 to 3d169e6 Compare May 14, 2024 18:55
@golobitch golobitch requested a review from njlie May 14, 2024 20:00
@golobitch
Copy link
Collaborator Author

@njlie @mkurapov I have also moved the Grant lookup to the interaction server.

@golobitch golobitch requested a review from mkurapov May 15, 2024 13:14
njlie
njlie previously approved these changes May 21, 2024
const acceptResponse = await fetch(
`${url.toString()}grant/${interactId}/${nonce}/accept`,
`${deps.sendingASE.config.interactionServer}/grant/${interactId}/${nonce}/accept`,
Copy link
Contributor

Choose a reason for hiding this comment

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

final thing, I think we just need to expose interactionServer on the TestConfig interface

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let me check that. I also need to rebase to main

@golobitch golobitch force-pushed the 2649-auth-move-interaction-routes-to-a-different-port-1 branch from a512b22 to c0fb04c Compare May 27, 2024 15:05
Accept and reject interaction choices should not be exposed. but other routes can and must be
exposed. This is why we need to move the choice routes to different port

BREAKING CHANGE: Routes for accepting and rejecting choice are no longer exposed. Ideally, this must
be done through ASE backend service that checks for authentication / authorization
@golobitch golobitch force-pushed the 2649-auth-move-interaction-routes-to-a-different-port-1 branch 3 times, most recently from 0d1037e to ecd799b Compare May 27, 2024 18:53
@golobitch golobitch force-pushed the 2649-auth-move-interaction-routes-to-a-different-port-1 branch from ecd799b to 4135dfa Compare May 27, 2024 18:59
@golobitch golobitch requested review from mkurapov and njlie May 27, 2024 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Issue/PR that introduces breaking changes pkg: auth Changes in the GNAP auth package. pkg: documentation Changes in the documentation package. pkg: mock-ase type: documentation Improvements or additions to documentation type: source Changes business logic type: tests Testing related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Auth] Move interaction routes to a different port
4 participants