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] Websockets authentication #291

Merged
merged 10 commits into from
Sep 4, 2024
Merged

Conversation

lukaskabc
Copy link
Collaborator

No description provided.

@@ -96,6 +86,17 @@ public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
return http.build();
}

/**
* An attempt to replicate auth provider from HttpSecurity
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rewrite this comment. Why is this an attempt? The comment should give definitive information about what the method does.

if (username == null || username.isBlank()) {
throw new JwtException("Invalid JWT token contents");
}
final TermItUserDetails existingDetails = userDetailsService.loadUserByUsername(username);
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems a bit weird (and wasteful) that user details are loaded by the TermItJwtDecoder.decode, thrown away and the loaded again here.

Copy link
Contributor

@ledsoft ledsoft left a comment

Choose a reason for hiding this comment

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

One last thing - please cleanup unused imports.

Copy link
Contributor

@ledsoft ledsoft left a comment

Choose a reason for hiding this comment

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

LGTM

@ledsoft ledsoft merged commit 79d4206 into kbss-cvut:development Sep 4, 2024
2 checks passed
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.

2 participants