-
Notifications
You must be signed in to change notification settings - Fork 4
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
DTS-34986 Central AuthNAuth Spring Boot upgrade #1012
Conversation
...thapi/src/main/java/com/publicissapient/kpidashboard/apis/controller/APITokenController.java
Outdated
Show resolved
Hide resolved
...i/src/main/java/com/publicissapient/kpidashboard/apis/service/SAMLAuthenticationService.java
Show resolved
Hide resolved
...src/main/java/com/publicissapient/kpidashboard/apis/service/dto/ResetPasswordRequestDTO.java
Show resolved
Hide resolved
…y and save saml details only the first time user logs in
…hecking the documentation
…y and save saml details only the first time user logs in
}else{ | ||
if(redirectUri.indexOf('?') === -1){ | ||
window.location.href = `${redirectUri}?authToken=${authToken}`; | ||
window.location.href = `${redirectUri}`; |
Check failure
Code scanning / CodeQL
Client-side cross-site scripting High
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 2 months ago
To fix the problem, we need to ensure that the redirectUri
is properly sanitized before it is used. This can be achieved by validating the redirectUri
against a whitelist of allowed URLs or by using a library to sanitize the URL.
- General Fix: Validate or sanitize the
redirectUri
before using it inwindow.location.href
. - Detailed Fix: Implement a function to validate the
redirectUri
and use this function before settingwindow.location.href
.
-
Copy modified line R14 -
Copy modified lines R39-R52
@@ -13,3 +13,3 @@ | ||
const storeUserDetails = useCallback((authToken) => { | ||
const redirectUri = JSON.parse(localStorage.getItem('redirect_uri')); | ||
const redirectUri = sanitizeRedirectUri(JSON.parse(localStorage.getItem('redirect_uri'))); | ||
apiProvider.getLoginStatus(authToken) | ||
@@ -38,2 +38,16 @@ | ||
|
||
const sanitizeRedirectUri = (uri) => { | ||
const allowedDomains = ['example.com', 'another-example.com']; // Add allowed domains here | ||
try { | ||
const url = new URL(uri); | ||
if (allowedDomains.includes(url.hostname)) { | ||
return uri; | ||
} else { | ||
return null; | ||
} | ||
} catch (e) { | ||
return null; | ||
} | ||
}; | ||
|
||
useEffect(() => { |
}else{ | ||
window.location.href = `${redirectUri}&authToken=${authToken}`; | ||
window.location.href = `${redirectUri}`; |
Check failure
Code scanning / CodeQL
Client-side cross-site scripting High
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 2 months ago
To fix the problem, we need to ensure that the redirectUri
is properly sanitized before it is used. One effective way to do this is to use a library like DOMPurify
to sanitize the URL. This will help prevent any malicious scripts from being executed.
- Install the
DOMPurify
library. - Import
DOMPurify
in the relevant files. - Use
DOMPurify
to sanitize theredirectUri
before using it.
-
Copy modified line R2 -
Copy modified lines R15-R16
@@ -1,2 +1,3 @@ | ||
import React, { useState, useEffect, useCallback } from 'react'; | ||
import DOMPurify from 'dompurify'; | ||
import apiProvider from '../../services/API/IndividualApis'; | ||
@@ -13,3 +14,4 @@ | ||
const storeUserDetails = useCallback((authToken) => { | ||
const redirectUri = JSON.parse(localStorage.getItem('redirect_uri')); | ||
let redirectUri = JSON.parse(localStorage.getItem('redirect_uri')); | ||
redirectUri = DOMPurify.sanitize(redirectUri); | ||
apiProvider.getLoginStatus(authToken) |
-
Copy modified lines R18-R19
@@ -17,3 +17,4 @@ | ||
"react-scripts": "5.0.1", | ||
"web-vitals": "^2.1.4" | ||
"web-vitals": "^2.1.4", | ||
"dompurify": "^3.1.7" | ||
}, |
-
Copy modified line R2 -
Copy modified lines R52-R53
@@ -1,2 +1,3 @@ | ||
import React, { useState, useEffect } from 'react'; | ||
import DOMPurify from 'dompurify'; | ||
import { NavLink } from 'react-router-dom'; | ||
@@ -50,3 +51,4 @@ | ||
if (redirectUri) { | ||
localStorage.setItem('redirect_uri', JSON.stringify(redirectUri)); | ||
const sanitizedRedirectUri = DOMPurify.sanitize(redirectUri); | ||
localStorage.setItem('redirect_uri', JSON.stringify(sanitizedRedirectUri)); | ||
} |
Package | Version | Security advisories |
dompurify (npm) | 3.1.7 | None |
}else{ | ||
if(redirectUri.indexOf('?') === -1){ | ||
window.location.href = `${redirectUri}?authToken=${authToken}`; | ||
window.location.href = `${redirectUri}`; |
Check warning
Code scanning / CodeQL
Client-side URL redirect Medium
user-provided value
Untrusted URL redirection depends on a
user-provided value
Untrusted URL redirection depends on a
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 2 months ago
To fix the problem, we need to validate the redirectUri
against a list of authorized URLs before performing the redirection. This ensures that only trusted URLs are used for redirection, mitigating the risk of phishing attacks.
- Create a list of authorized URLs: Define a list of URLs that are considered safe for redirection.
- Validate the
redirectUri
: Before using theredirectUri
for redirection, check if it is in the list of authorized URLs. - Perform the redirection: Only redirect if the
redirectUri
is validated against the authorized list.
-
Copy modified lines R15-R18 -
Copy modified line R24 -
Copy modified line R27
@@ -14,2 +14,6 @@ | ||
const redirectUri = JSON.parse(localStorage.getItem('redirect_uri')); | ||
const authorizedUrls = [ | ||
'https://trusted.example.com', | ||
'https://another-trusted.example.com' | ||
]; | ||
apiProvider.getLoginStatus(authToken) | ||
@@ -19,10 +23,6 @@ | ||
let defaultAppUrl = process.env.NODE_ENV === 'production' ? window.env.REACT_APP_PSKnowHOW : process.env.REACT_APP_PSKnowHOW; | ||
if(!redirectUri){ | ||
if(!redirectUri || !authorizedUrls.includes(new URL(redirectUri).origin)){ | ||
window.location.href = defaultAppUrl; | ||
}else{ | ||
if(redirectUri.indexOf('?') === -1){ | ||
window.location.href = `${redirectUri}`; | ||
}else{ | ||
window.location.href = `${redirectUri}`; | ||
} | ||
window.location.href = redirectUri; | ||
} |
}else{ | ||
window.location.href = `${redirectUri}&authToken=${authToken}`; | ||
window.location.href = `${redirectUri}`; |
Check warning
Code scanning / CodeQL
Client-side URL redirect Medium
user-provided value
Untrusted URL redirection depends on a
user-provided value
Untrusted URL redirection depends on a
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 2 months ago
To fix the problem, we need to ensure that the redirectUri
is validated against a list of authorized URLs or domains before performing the redirection. This can be achieved by maintaining a whitelist of allowed redirect URLs and checking if the redirectUri
belongs to this list before using it.
- Create a whitelist of authorized redirect URLs.
- Validate the
redirectUri
against this whitelist. - Only perform the redirection if the
redirectUri
is in the whitelist; otherwise, redirect to a default safe URL.
-
Copy modified lines R15-R19 -
Copy modified line R25 -
Copy modified lines R27-R28
@@ -14,2 +14,7 @@ | ||
const redirectUri = JSON.parse(localStorage.getItem('redirect_uri')); | ||
const authorizedRedirects = [ | ||
'https://trusted-domain1.com', | ||
'https://trusted-domain2.com', | ||
// Add more authorized URLs here | ||
]; | ||
apiProvider.getLoginStatus(authToken) | ||
@@ -19,10 +24,6 @@ | ||
let defaultAppUrl = process.env.NODE_ENV === 'production' ? window.env.REACT_APP_PSKnowHOW : process.env.REACT_APP_PSKnowHOW; | ||
if(!redirectUri){ | ||
if (!redirectUri || !authorizedRedirects.includes(new URL(redirectUri).origin)) { | ||
window.location.href = defaultAppUrl; | ||
}else{ | ||
if(redirectUri.indexOf('?') === -1){ | ||
window.location.href = `${redirectUri}`; | ||
}else{ | ||
window.location.href = `${redirectUri}`; | ||
} | ||
} else { | ||
window.location.href = redirectUri; | ||
} |
* @throws ParseException | ||
*/ | ||
private void apiCallToGetBranches(List<BambooBranchesResponseDTO> responseDTOList, String url, HttpEntity<?> httpEntity) throws ParseException { | ||
ResponseEntity<String> response = restTemplate.exchange(url, HttpMethod.GET, httpEntity, String.class); |
Check failure
Code scanning / CodeQL
Server-side request forgery Critical
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 2 months ago
To fix the SSRF vulnerability, we need to ensure that the jobNameKey
parameter is validated against a list of known, safe values or ensure that the constructed URL is limited to a particular host or more restrictive URL prefix. Here, we will implement a whitelist approach for jobNameKey
.
- Create a whitelist of valid
jobNameKey
values: This can be a predefined list of acceptable keys. - Validate
jobNameKey
against the whitelist: Before constructing the URL, check ifjobNameKey
is in the whitelist. - Reject invalid
jobNameKey
values: IfjobNameKey
is not in the whitelist, log an error and do not proceed with the request.
-
Copy modified lines R114-R121
@@ -113,2 +113,10 @@ | ||
|
||
// Whitelist of valid jobNameKey values | ||
List<String> validJobNameKeys = List.of("validKey1", "validKey2", "validKey3"); | ||
|
||
if (!validJobNameKeys.contains(jobNameKey)) { | ||
log.error("Invalid jobNameKey: {}", jobNameKey); | ||
return responseDTOList; | ||
} | ||
|
||
String url = String.format(new StringBuilder(baseUrl).append(RESOURCE_BRANCH_ENDPOINT).toString(), |
String statusCode = response.getStatusCode().toString(); | ||
log.error("Error while fetching BambooBranchesNameAndKeys from {}. with status {}", url, | ||
statusCode); | ||
log.error("Invalid jobNameKey: {}", jobNameKey); |
Check failure
Code scanning / CodeQL
Log Injection High
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 2 months ago
To fix the log injection issue, we need to sanitize the jobNameKey
before logging it. The best way to do this is to remove any newline characters and other potentially harmful characters from the jobNameKey
. We can use the String.replace
method to replace newline characters with an empty string. Additionally, we should ensure that the jobNameKey
is clearly marked in the log entry to prevent any confusion.
-
Copy modified lines R123-R124
@@ -122,3 +122,4 @@ | ||
} else { | ||
log.error("Invalid jobNameKey: {}", jobNameKey); | ||
String sanitizedJobNameKey = jobNameKey.replace("\n", "").replace("\r", ""); | ||
log.error("Invalid jobNameKey: {}", sanitizedJobNameKey); | ||
} |
parseBranchesResponse(responseDTOList, response); | ||
} else { | ||
String statusCode = response.getStatusCode().toString(); | ||
log.error("Error while fetching BambooBranchesNameAndKeys from {}. with status {}", url, |
Check failure
Code scanning / CodeQL
Log Injection High
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 2 months ago
To fix the log injection issue, we need to sanitize the user input before logging it. Specifically, we should remove any potentially harmful characters from the url
parameter before it is logged. This can be done by replacing new-line characters and other special characters with safe alternatives.
- Sanitize the
url
parameter by replacing new-line characters and other special characters. - Update the log statement to use the sanitized
url
.
-
Copy modified lines R147-R148
@@ -146,3 +146,4 @@ | ||
String statusCode = response.getStatusCode().toString(); | ||
log.error("Error while fetching BambooBranchesNameAndKeys from {}. with status {}", url, | ||
String sanitizedUrl = url.replaceAll("[\\r\\n]", "_"); | ||
log.error("Error while fetching BambooBranchesNameAndKeys from {}. with status {}", sanitizedUrl, | ||
statusCode); |
try { | ||
calculateAllKPIAggregatedMetrics(kpiRequest, responseList, kpiElement, treeAggregatorDetail); | ||
} catch (Exception e) { | ||
log.error("Error while KPI calculation for data {}", kpiRequest.getKpiList(), e); |
Check failure
Code scanning / CodeQL
Log Injection High
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 2 months ago
To fix the log injection issue, we need to sanitize the user-provided data before logging it. Specifically, we should remove any potentially harmful characters such as newlines that could be used to forge log entries. We can achieve this by replacing newline characters with an empty string or a space.
- Identify the log statement that uses
kpiRequest.getKpiList()
in theJenkinsServiceR
class. - Sanitize the
kpiRequest.getKpiList()
data by removing newline characters before logging it. - Ensure that the sanitization does not alter the functionality of the code.
-
Copy modified lines R124-R125
@@ -123,3 +123,4 @@ | ||
} catch (Exception e) { | ||
log.error("Error while KPI calculation for data {}", kpiRequest.getKpiList(), e); | ||
String sanitizedKpiList = kpiRequest.getKpiList().toString().replace("\n", "").replace("\r", ""); | ||
log.error("Error while KPI calculation for data {}", sanitizedKpiList, e); | ||
} |
...ce/authapi/src/main/java/com/publicissapient/kpidashboard/apis/config/WebSecurityConfig.java
Dismissed
Show dismissed
Hide dismissed
...va/com/publicissapient/kpidashboard/apis/service/impl/StandardAuthenticationServiceImpl.java
Fixed
Show fixed
Hide fixed
...va/com/publicissapient/kpidashboard/apis/service/impl/StandardAuthenticationServiceImpl.java
Fixed
Show fixed
Hide fixed
...va/com/publicissapient/kpidashboard/apis/service/impl/StandardAuthenticationServiceImpl.java
Fixed
Show fixed
Hide fixed
...uthapi/src/main/java/com/publicissapient/kpidashboard/apis/service/impl/UserServiceImpl.java
Fixed
Show fixed
Hide fixed
...uthapi/src/main/java/com/publicissapient/kpidashboard/apis/service/impl/UserServiceImpl.java
Fixed
Show fixed
Hide fixed
…hecking the documentation
…y and save saml details only the first time user logs in
…y and save saml details only the first time user logs in
…rade-final-AuthUI-Dockerfile changing permission in dockerfile auth ui
update auth.baseUrl Signed-off-by: Hirenkumar babariya <99163630+hirbabar@users.noreply.github.com>
# Conflicts: # central-auth-service/central-login-ui-react/src/pages/login/index.jsx
Signed-off-by: rapkalya <74697698+rapkalya@users.noreply.github.com>
Signed-off-by: rapkalya <74697698+rapkalya@users.noreply.github.com>
Signed-off-by: rapkalya <74697698+rapkalya@users.noreply.github.com>
Signed-off-by: rapkalya <74697698+rapkalya@users.noreply.github.com>
Signed-off-by: rapkalya <74697698+rapkalya@users.noreply.github.com>
Signed-off-by: rapkalya <74697698+rapkalya@users.noreply.github.com>
Signed-off-by: rapkalya <74697698+rapkalya@users.noreply.github.com>
Signed-off-by: rapkalya <74697698+rapkalya@users.noreply.github.com>
Signed-off-by: rapkalya <74697698+rapkalya@users.noreply.github.com>
Signed-off-by: rapkalya <74697698+rapkalya@users.noreply.github.com>
this PR changes is merged in #1520 PR. so not reqied this PR so that i am closeing this PR |
Upgrading the Central AuthNAuth Spring Boot version to 3.2 and Java version to 17.
For full documentation of the upgrade, you can check the confluence page:
https://publicissapient.atlassian.net/wiki/spaces/SPDS/pages/307789878/Java+17+Upgrade