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(jans-auth-server): new jans server installation show null in place of client_name #9415 #9523

Merged
merged 2 commits into from
Sep 19, 2024

Conversation

yuriyz
Copy link
Contributor

@yuriyz yuriyz commented Sep 19, 2024

Description

fix(jans-auth-server): new jans server installation show null in place of client_name

Target issue

closes #9415

Test and Document the changes

  • Static code analysis has been run locally and issues have been fixed
  • Tested manually

Please check the below before submitting your PR. The PR will not be merged if there are no commits that start with docs: to indicate documentation changes or if the below checklist is not selected.

  • I confirm that there is no impact on the docs due to the code changes in this PR.

Copy link

dryrunsecurity bot commented Sep 19, 2024

DryRun Security Summary

The pull request focuses on improving the user experience and client representation during the authorization flow in the Jans Auth Server application, with changes to the AuthorizeAction class and the authorize-extended-template.xhtml file, while emphasizing the importance of reviewing the entire codebase to ensure secure implementation of authorization-related functionality and proper sanitization and validation of the client display name to prevent potential security vulnerabilities.

Expand for full summary

Summary:

The code changes in this pull request are focused on improving the user experience and client representation during the authorization flow in the Jans Auth Server application. The key changes include the addition of the getClientDisplayName() method in the AuthorizeAction class, which retrieves the client's display name or ID if the name is not available, and a UI-level change in the authorize-extended-template.xhtml file to display a more user-friendly client name.

From an application security perspective, the changes do not introduce any major security concerns. However, it is essential to review the entire codebase and ensure that all authorization-related functionality is implemented securely, including proper validation of input parameters, secure handling of session management, careful management of sensitive information, robust error handling and logging, and regular security audits and penetration testing. Additionally, the client display name should be properly sanitized and validated to prevent potential Cross-Site Scripting (XSS) vulnerabilities.

Files Changed:

  1. jans-auth-server/server/src/main/java/io/jans/as/server/authorize/ws/rs/AuthorizeAction.java:

    • The changes include the addition of the getClientDisplayName() method, which retrieves the client's display name or ID if the name is not available. This helps to provide a user-friendly representation of the client in the authorization flow.
    • From a security perspective, the changes do not introduce any major security concerns, but it's important to review the entire codebase and ensure that all authorization-related functionality is implemented securely.
  2. jans-auth-server/server/src/main/webapp/WEB-INF/incl/layout/authorize-extended-template.xhtml:

    • The changes replace the previous logic, which used client.getClientName() or client.getClientId() to display the client name, with a new property authorizeAction.clientDisplayName.
    • While this change is not immediately concerning from a security perspective, it's important to ensure that the authorizeAction.clientDisplayName property is properly sanitized and validated to prevent potential Cross-Site Scripting (XSS) vulnerabilities.

Code Analysis

We ran 9 analyzers against 2 files and 1 analyzer had findings. 8 analyzers had no findings.

Analyzer Findings
Authn/Authz Analyzer 1 finding

Riskiness

🟢 Risk threshold not exceeded.

View PR in the DryRun Dashboard.

@yuriyz
Copy link
Contributor Author

yuriyz commented Sep 19, 2024

image

@yuriyz yuriyz enabled auto-merge (squash) September 19, 2024 09:59
@mo-auto mo-auto added comp-jans-auth-server Component affected by issue or PR kind-bug Issue or PR is a bug in existing functionality labels Sep 19, 2024
Copy link

sonarcloud bot commented Sep 19, 2024

@yuriyz yuriyz merged commit 23ef80c into main Sep 19, 2024
12 checks passed
@yuriyz yuriyz deleted the jans-auth-server-9415 branch September 19, 2024 11:08
yuriyz added a commit that referenced this pull request Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp-jans-auth-server Component affected by issue or PR kind-bug Issue or PR is a bug in existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix(jans-auth-server): new jans server installation show null in place of client_name
4 participants