Skip to content

Update *.java style for security/action/* #2824

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.

Sign up for GitHub

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

Closed
stephen-crawford wants to merge 4 commits into from
Closed

Update *.java style for security/action/* #2824

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0185557
rebase
stephen-crawford May 31, 2023
43fa8b8
Update java style under **/auth
stephen-crawford May 31, 2023
71cbd69
Update Util dir
stephen-crawford May 31, 2023
d72bd11
Update Security/Action/*
stephen-crawford May 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 53 additions & 2 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
*/


import com.diffplug.gradle.spotless.JavaExtension
import org.opensearch.gradle.test.RestIntegTestTask

buildscript {
Expand Down Expand Up @@ -51,7 +52,7 @@ plugins {
id 'idea'
id 'jacoco'
id 'maven-publish'
id 'com.diffplug.spotless' version '6.18.0'
id 'com.diffplug.spotless' version '6.19.0'
id 'checkstyle'
id 'com.netflix.nebula.ospackage' version "11.1.0"
id "org.gradle.test-retry" version "1.5.2"
Expand All @@ -69,7 +70,57 @@ apply plugin: 'opensearch.opensearchplugin'
apply plugin: 'opensearch.pluginzip'
apply plugin: 'opensearch.rest-test'
apply plugin: 'opensearch.testclusters'
apply from: 'gradle/formatting.gradle'

spotless {
java {
// Normally this isn't necessary, but we have Java sources in
// non-standard places
target '**/com/amazon/dlic/**/*.java'
target '**/com/amazon/security/action/**/*.java'

removeUnusedImports()
eclipse().configFile rootProject.file('formatter/formatterConfig.xml')
trimTrailingWhitespace()
endWithNewline();

// note: you can use an empty string for all the imports you didn't specify explicitly, and '\\#` prefix for static imports
importOrder('java', 'javax', '', 'com.amazon', 'org.opensearch', '\\#')

custom 'Refuse wildcard imports', {
// Wildcard imports can't be resolved; fail the build
if (it =~ /\s+import .*\*;/) {
throw new AssertionError("Do not use wildcard imports. 'spotlessApply' cannot resolve this issue.")
}
}

// See DEVELOPER_GUIDE.md for details of when to enable this.
if (System.getProperty('spotless.paddedcell') != null) {
paddedCell()
}
}
format 'misc', {
target '*.md', '*.gradle', '**/*.json', '**/*.yaml', '**/*.yml', '**/*.svg'

trimTrailingWhitespace()
endWithNewline()
}
format('javaFoo', JavaExtension) {

importOrder('java', 'javax', '', 'com.amazon', 'org.opensearch', '\\#')
target '**/*.java'
targetExclude '**/com/amazon/dlic/**/*.java'
targetExclude '**/com/amazon/security/action/**/*.java'
targetExclude('src/integrationTest/**')

trimTrailingWhitespace()
endWithNewline();
}
format("integrationTest", JavaExtension) {
target('src/integrationTest/java/**/*.java')
importOrder('java', 'javax', '', 'com.amazon', 'org.opensearch', '\\#')
indentWithTabs(4)
}
}

licenseFile = rootProject.file('LICENSE.txt')
noticeFile = rootProject.file('NOTICE.txt')
Expand Down
8 changes: 4 additions & 4 deletions bwc-test/src/test/java/SecurityBackwardsCompatibilityIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,17 @@
import java.util.Set;
import java.util.stream.Collectors;

import org.junit.Assume;
import com.google.common.collect.ImmutableMap;
import org.junit.Assert;
import org.junit.Assume;
import org.junit.Before;

import org.opensearch.Version;
import org.opensearch.client.Response;
import org.opensearch.common.settings.Settings;
import org.opensearch.rest.RestStatus;
import org.opensearch.test.rest.OpenSearchRestTestCase;

import org.opensearch.Version;
import com.google.common.collect.ImmutableMap;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.hasItem;

Expand Down
36 changes: 0 additions & 36 deletions gradle/formatting.gradle
View file
Open in desktop

This file was deleted.

24 changes: 15 additions & 9 deletions src/main/java/com/amazon/dlic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public abstract class AbstractHTTPJwtAuthenticator implements HTTPAuthenticator
private final String requiredIssuer;

public static final int DEFAULT_CLOCK_SKEW_TOLERANCE_SECONDS = 30;
private final int clockSkewToleranceSeconds ;
private final int clockSkewToleranceSeconds;

public AbstractHTTPJwtAuthenticator(Settings settings, Path configPath) {
jwtUrlParameter = settings.get("jwt_url_parameter");
Expand All @@ -83,8 +83,7 @@ public AbstractHTTPJwtAuthenticator(Settings settings, Path configPath) {

@Override
@SuppressWarnings("removal")
public AuthCredentials extractCredentials(RestRequest request, ThreadContext context)
throws OpenSearchSecurityException {
public AuthCredentials extractCredentials(RestRequest request, ThreadContext context) throws OpenSearchSecurityException {
final SecurityManager sm = System.getSecurityManager();

if (sm != null) {
Expand Down Expand Up @@ -186,8 +185,11 @@ public String extractSubject(JwtClaims claims) {
// warning
if (!(subjectObject instanceof String)) {
log.warn(
"Expected type String for roles in the JWT for subject_key {}, but value was '{}' ({}). Will convert this value to String.",
subjectKey, subjectObject, subjectObject.getClass());
"Expected type String for roles in the JWT for subject_key {}, but value was '{}' ({}). Will convert this value to String.",
subjectKey,
subjectObject,
subjectObject.getClass()
);
subject = String.valueOf(subjectObject);
} else {
subject = (String) subjectObject;
Expand All @@ -207,8 +209,9 @@ public String[] extractRoles(JwtClaims claims) {

if (rolesObject == null) {
log.warn(
"Failed to get roles from JWT claims with roles_key '{}'. Check if this key is correct and available in the JWT payload.",
rolesKey);
"Failed to get roles from JWT claims with roles_key '{}'. Check if this key is correct and available in the JWT payload.",
rolesKey
);
return new String[0];
}

Expand All @@ -218,8 +221,11 @@ public String[] extractRoles(JwtClaims claims) {
// String but issue a warning
if (!(rolesObject instanceof String) && !(rolesObject instanceof Collection<?>)) {
log.warn(
"Expected type String or Collection for roles in the JWT for roles_key {}, but value was '{}' ({}). Will convert this value to String.",
rolesKey, rolesObject, rolesObject.getClass());
"Expected type String or Collection for roles in the JWT for roles_key {}, but value was '{}' ({}). Will convert this value to String.",
rolesKey,
rolesObject,
rolesObject.getClass()
);
} else if (rolesObject instanceof Collection<?>) {
roles = ((Collection<String>) rolesObject).toArray(new String[0]);
}
Expand Down
113 changes: 65 additions & 48 deletions src/main/java/com/amazon/dlic/auth/http/jwt/HTTPJwtAuthenticator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public HTTPJwtAuthenticator(final Settings settings, final Path configPath) {
try {
String signingKey = settings.get("signing_key");

if(signingKey == null || signingKey.length() == 0) {
if (signingKey == null || signingKey.length() == 0) {
log.error("signingKey must not be null or empty. JWT authentication will not work");
} else {

Expand All @@ -90,7 +90,7 @@ public HTTPJwtAuthenticator(final Settings settings, final Path configPath) {
log.debug("No public ECDSA key, try other algos ({})", e.toString());
}

if(key != null) {
if (key != null) {
_jwtParser = Jwts.parser().setSigningKey(key);
} else {
_jwtParser = Jwts.parser().setSigningKey(decoded);
Expand Down Expand Up @@ -121,7 +121,6 @@ public HTTPJwtAuthenticator(final Settings settings, final Path configPath) {
jwtParser = _jwtParser;
}


@Override
@SuppressWarnings("removal")
public AuthCredentials extractCredentials(RestRequest request, ThreadContext context) throws OpenSearchSecurityException {
Expand Down Expand Up @@ -152,25 +151,29 @@ private AuthCredentials extractCredentials0(final RestRequest request) {
jwtToken = null;
}

if((jwtToken == null || jwtToken.isEmpty()) && jwtUrlParameter != null) {
if ((jwtToken == null || jwtToken.isEmpty()) && jwtUrlParameter != null) {
jwtToken = request.param(jwtUrlParameter);
} else {
//just consume to avoid "contains unrecognized parameter"
// just consume to avoid "contains unrecognized parameter"
request.param(jwtUrlParameter);
}

if (jwtToken == null || jwtToken.length() == 0) {
if(log.isDebugEnabled()) {
log.debug("No JWT token found in '{}' {} header", jwtUrlParameter==null?jwtHeaderName:jwtUrlParameter, jwtUrlParameter==null?"header":"url parameter");
if (log.isDebugEnabled()) {
log.debug(
"No JWT token found in '{}' {} header",
jwtUrlParameter == null ? jwtHeaderName : jwtUrlParameter,
jwtUrlParameter == null ? "header" : "url parameter"
);
}
return null;
}

final int index;
if((index = jwtToken.toLowerCase().indexOf(BEARER)) > -1) { //detect Bearer
jwtToken = jwtToken.substring(index+BEARER.length());
if ((index = jwtToken.toLowerCase().indexOf(BEARER)) > -1) { // detect Bearer
jwtToken = jwtToken.substring(index + BEARER.length());
} else {
if(log.isDebugEnabled()) {
if (log.isDebugEnabled()) {
log.debug("No Bearer scheme found in header");
}
}
Expand All @@ -181,16 +184,16 @@ private AuthCredentials extractCredentials0(final RestRequest request) {
final String subject = extractSubject(claims, request);

if (subject == null) {
log.error("No subject found in JWT token");
return null;
log.error("No subject found in JWT token");
return null;
}

final String[] roles = extractRoles(claims, request);

final AuthCredentials ac = new AuthCredentials(subject, roles).markComplete();

for(Entry<String, Object> claim: claims.entrySet()) {
ac.addAttribute("attr.jwt."+claim.getKey(), String.valueOf(claim.getValue()));
for (Entry<String, Object> claim : claims.entrySet()) {
ac.addAttribute("attr.jwt." + claim.getKey(), String.valueOf(claim.getValue()));
}

return ac;
Expand All @@ -199,7 +202,7 @@ private AuthCredentials extractCredentials0(final RestRequest request) {
log.error("Cannot authenticate user with JWT because of ", e);
return null;
} catch (Exception e) {
if(log.isDebugEnabled()) {
if (log.isDebugEnabled()) {
log.debug("Invalid or expired JWT token.", e);
}
return null;
Expand All @@ -208,7 +211,7 @@ private AuthCredentials extractCredentials0(final RestRequest request) {

@Override
public boolean reRequestAuthentication(final RestChannel channel, AuthCredentials creds) {
final BytesRestResponse wwwAuthenticateResponse = new BytesRestResponse(RestStatus.UNAUTHORIZED,"");
final BytesRestResponse wwwAuthenticateResponse = new BytesRestResponse(RestStatus.UNAUTHORIZED, "");
wwwAuthenticateResponse.addHeader("WWW-Authenticate", "Bearer realm=\"OpenSearch Security\"");
channel.sendResponse(wwwAuthenticateResponse);
return true;
Expand All @@ -221,16 +224,21 @@ public String getType() {

protected String extractSubject(final Claims claims, final RestRequest request) {
String subject = claims.getSubject();
if(subjectKey != null) {
// try to get roles from claims, first as Object to avoid having to catch the ExpectedTypeException
if (subjectKey != null) {
// try to get roles from claims, first as Object to avoid having to catch the ExpectedTypeException
Object subjectObject = claims.get(subjectKey, Object.class);
if(subjectObject == null) {
if (subjectObject == null) {
log.warn("Failed to get subject from JWT claims, check if subject_key '{}' is correct.", subjectKey);
return null;
}
// We expect a String. If we find something else, convert to String but issue a warning
if(!(subjectObject instanceof String)) {
log.warn("Expected type String for roles in the JWT for subject_key {}, but value was '{}' ({}). Will convert this value to String.", subjectKey, subjectObject, subjectObject.getClass());
// We expect a String. If we find something else, convert to String but issue a warning
if (!(subjectObject instanceof String)) {
log.warn(
"Expected type String for roles in the JWT for subject_key {}, but value was '{}' ({}). Will convert this value to String.",
subjectKey,
subjectObject,
subjectObject.getClass()
);
}
subject = String.valueOf(subjectObject);
}
Expand All @@ -239,34 +247,43 @@ protected String extractSubject(final Claims claims, final RestRequest request)

@SuppressWarnings("unchecked")
protected String[] extractRoles(final Claims claims, final RestRequest request) {
// no roles key specified
if(rolesKey == null) {
return new String[0];
}
// try to get roles from claims, first as Object to avoid having to catch the ExpectedTypeException
final Object rolesObject = claims.get(rolesKey, Object.class);
if(rolesObject == null) {
log.warn("Failed to get roles from JWT claims with roles_key '{}'. Check if this key is correct and available in the JWT payload.", rolesKey);
return new String[0];
}

String[] roles = String.valueOf(rolesObject).split(",");

// We expect a String or Collection. If we find something else, convert to String but issue a warning
if (!(rolesObject instanceof String) && !(rolesObject instanceof Collection<?>)) {
log.warn("Expected type String or Collection for roles in the JWT for roles_key {}, but value was '{}' ({}). Will convert this value to String.", rolesKey, rolesObject, rolesObject.getClass());
} else if (rolesObject instanceof Collection<?>) {
roles = ((Collection<String>) rolesObject).toArray(new String[0]);
}

for (int i = 0; i < roles.length; i++) {
roles[i] = roles[i].trim();
}

return roles;
// no roles key specified
if (rolesKey == null) {
return new String[0];
}
// try to get roles from claims, first as Object to avoid having to catch the ExpectedTypeException
final Object rolesObject = claims.get(rolesKey, Object.class);
if (rolesObject == null) {
log.warn(
"Failed to get roles from JWT claims with roles_key '{}'. Check if this key is correct and available in the JWT payload.",
rolesKey
);
return new String[0];
}

String[] roles = String.valueOf(rolesObject).split(",");

// We expect a String or Collection. If we find something else, convert to String but issue a warning
if (!(rolesObject instanceof String) && !(rolesObject instanceof Collection<?>)) {
log.warn(
"Expected type String or Collection for roles in the JWT for roles_key {}, but value was '{}' ({}). Will convert this value to String.",
rolesKey,
rolesObject,
rolesObject.getClass()
);
} else if (rolesObject instanceof Collection<?>) {
roles = ((Collection<String>) rolesObject).toArray(new String[0]);
}

for (int i = 0; i < roles.length; i++) {
roles[i] = roles[i].trim();
}

return roles;
}

private static PublicKey getPublicKey(final byte[] keyBytes, final String algo) throws NoSuchAlgorithmException, InvalidKeySpecException {
private static PublicKey getPublicKey(final byte[] keyBytes, final String algo) throws NoSuchAlgorithmException,
InvalidKeySpecException {
X509EncodedKeySpec spec = new X509EncodedKeySpec(keyBytes);
KeyFactory kf = KeyFactory.getInstance(algo);
return kf.generatePublic(spec);
Expand Down
Loading