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

IMN-521 - Replace Fastify with Express in authorization-server #1180

Merged
merged 20 commits into from
Nov 18, 2024

Conversation

shuyec
Copy link
Contributor

@shuyec shuyec commented Nov 7, 2024

Merge after #1086

Replaced Fastify with Express in authorization-server

Copy link
Contributor

@galales galales left a comment

Choose a reason for hiding this comment

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

🚀
minor: avoid using refactor in urls because the environment will be dismissed and it may lead to confusion. In tests it is preferable to use dummy values

@@ -1 +1 @@
CLIENT_ASSERTION_AUDIENCE="test.interop.pagopa.it,dev.interop.pagopa.it"
CLIENT_ASSERTION_AUDIENCE="auth.refactor.dev.interop.pagopa.it/client-assertion,auth.refactor.test.interop.pagopa.it/client-assertion"
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure about the scope, but refactor.test is not used

@@ -109,6 +109,8 @@ export function tokenServiceBuilder({
verifyClientAssertion(request.client_assertion, request.client_id);

if (clientAssertionErrors) {
// TODO double check if errors have to be logged or put inside the error below (check the same for parameters errors)
logger.warn(clientAssertionErrors.map((error) => error.detail));
Copy link
Contributor

Choose a reason for hiding this comment

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

the error message should already be logged (I don't recall, in makeApiProblem maybe), so we can put the errors in the exception and use the standard flow

@taglioni-r taglioni-r merged commit 239becb into IMN-521_authorization-server Nov 18, 2024
49 checks passed
@taglioni-r taglioni-r deleted the IMN-521_fastify-to-express branch November 18, 2024 16:36
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.

3 participants