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

(db-authorization): fix a bug in handling jwt namespace option #2355

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

Conversation

mirko-pira
Copy link
Contributor

The update is designed to streamline and enhance the robustness of the jwt namespace handling mechanism, thereby eradicating any inconsistencies and potential formatting issues that may arise from improper namespace configuration.

Regardless of whether the "authorization" → "jwt" → "namespace" object is assigned a URL with or without a trailing slash, it will now be correctly work.

🔧 fix(db-authorization): Correct the jwt namespace option handling
The jwt namespace option within the authentication configuration now automatically appends a forward slash if missing. This modification ensures a uniform format across the board, eliminating any ambiguity and enhancing the system's reliability when parsing or utilizing the namespace.

💡 refactor(authentication.js): Enforce trailing slash for jwt namespace
The jwt namespace option's handling has been refined to include a trailing slash, if not already present, by modifying the authentication logic. This adjustment guarantees that the namespace is consistently formatted, preventing errors related to incorrect path concatenation or resource identification.

🔎 Use-case:
Previously, developers might encounter issues if the jwt namespace option did not conclude with a forward slash, leading to potential mishandling of resource paths or authorization checks. With the current update, specifying the jwt namespace in the authentication configuration object automatically ensures the inclusion of a trailing slash, thus standardizing the format and eliminating related errors. (check screenshot)

image

The code now correctly handles the jwt namespace option by ensuring that it always ends with a forward slash. This fix ensures consistency and prevents potential issues with the namespace configuration.

Signed-off-by: Mirko Pira <mp@mirkodev.com>
Signed-off-by: Mirko Pira <mp@mirkodev.com>
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

Thanks for opening a PR! Can you please add a unit test?

@mirko-pira
Copy link
Contributor Author

Thanks for opening a PR! Can you please add a unit test?

Here I am.
@mcollina should the unit test check the configuration or the "end results" of the X-PLATFORMATIC variables?

I've attempted to access the JWT configuration programmatically but can't seem to find a way.

@mcollina
Copy link
Member

The end result

@mirko-pira
Copy link
Contributor Author

mirko-pira commented Apr 25, 2024

After investigating the code more deeply, I realized that when the JWT session for the user is created by the 'fastify-user' plugin, it does not consider whether the namespace has a trailing slash.

For this reason, I think the best solution in this case is to cancel/close this PR, and I will open another one in the 'fastify-user' plugin repository, so we can address the issue at its source.

Does that sound like a good idea to you? :) (see screenshot for details)

EDIT: opened: platformatic/fastify-user#228

image

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.

None yet

2 participants