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

[Feature/Identity] Adds Basic Auth mechanism via Internal IdP #4798

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
66 commits
Select commit Hold shift + click to select a range
eae97da
Experiment with creating internal realm in OS
cwperks Sep 29, 2022
283ec5c
Create realm, read users from a file and show authentication flow wor…
cwperks Sep 30, 2022
68fb702
Add experimental tag in new classes
cwperks Sep 30, 2022
fe5a468
Run updateShas
cwperks Oct 3, 2022
2aaccf0
Create internal idp as a module
cwperks Oct 3, 2022
a154fdb
Add licenses/ directory for internal authn module
cwperks Oct 3, 2022
047bbea
Run spotlessApply
cwperks Oct 3, 2022
306f590
Add shiro-core LICENSE and NOTICE
cwperks Oct 3, 2022
4a00f72
Add missing licenses
cwperks Oct 3, 2022
b57d7b2
Add thirdPartyAudit
cwperks Oct 3, 2022
6d78226
Add naive internalClusterTest and yamlRestTest
cwperks Oct 3, 2022
3d315df
Fix yamlRestTest test case
cwperks Oct 3, 2022
69d6acd
Remove added line from security.policy
cwperks Oct 4, 2022
ad58ff5
Create idp in a sandbox library
cwperks Oct 4, 2022
b87adb7
Add jackson-dataformat-yaml sha1
cwperks Oct 4, 2022
2134f73
Start addressing code review comments and work on bundling issues
cwperks Oct 5, 2022
430cc9b
Add gradle property that enables native authn feature
cwperks Oct 6, 2022
c652fa2
Integrate new changes with existing identity classes
cwperks Oct 6, 2022
f29ce16
Apply spotless changes
cwperks Oct 6, 2022
ad66c3a
Disable jar hell in server/build.gradle to get beyond bcprov/bcfips l…
cwperks Oct 7, 2022
77c543e
Add jarHell.enabled = false to plugin-cli build.gradle
cwperks Oct 7, 2022
78e4ac1
Switch from InternalSubject to User and iterate on builder implementa…
cwperks Oct 10, 2022
bfb6e6a
Initial commit for basic auth handling via internal realm
DarshitChanpura Oct 12, 2022
abf05d3
Temp removes the system get property check for sandbox flag
DarshitChanpura Oct 13, 2022
553788d
Removes login method from subject interface as authentication is hand…
DarshitChanpura Oct 14, 2022
5ae8043
Removes authentication handling from RestController class
DarshitChanpura Oct 14, 2022
bb97c92
Adds javadoc comments and TODO statements to SecurityRestFilter and I…
DarshitChanpura Oct 14, 2022
faae7f7
Adds SecurityRestFilter class to ActionModule to enable REST Request …
DarshitChanpura Oct 14, 2022
a2e68bf
Grants Jackson databind Reflect access and makes changes to access Us…
DarshitChanpura Oct 14, 2022
27422fe
Adds internal realm instance to Node class and adds a failure respons…
DarshitChanpura Oct 14, 2022
d09309c
Fixes Spotless errors
DarshitChanpura Oct 14, 2022
7597e5b
Adds this PR to changelog file
DarshitChanpura Oct 14, 2022
c529707
Updates server build gradle to remove shiro dependency
DarshitChanpura Oct 14, 2022
b988ae2
Fixes forbiddenApis exception
DarshitChanpura Oct 14, 2022
96c28ca
Add token authentication flow
peternied Oct 18, 2022
d87f401
Updates login mechanism for Internal Subject
DarshitChanpura Nov 1, 2022
37c51a9
Removes login flow from InternalRealm
DarshitChanpura Nov 1, 2022
eee71cc
Resolves compileJava failure and adds new constructor for InternalAut…
DarshitChanpura Nov 1, 2022
a1ae88a
removes unneeded if clauses
DarshitChanpura Nov 2, 2022
ac4aab4
Adds authentication logic to RestController class
DarshitChanpura Nov 2, 2022
9dc87b1
Fixes spotless errors
DarshitChanpura Nov 2, 2022
1403ad2
Removes SecurityRestFilter class from this branch
DarshitChanpura Nov 2, 2022
ba32681
Adds a simple TODO comment on RestController
DarshitChanpura Nov 2, 2022
ae3ee0b
Fixes spotless errors
DarshitChanpura Nov 2, 2022
d6bed20
Modifies RestController to continue with request flow in case authent…
DarshitChanpura Nov 2, 2022
a34fcc9
Modifies Node class to use internal auth manager instead of noop auth…
DarshitChanpura Nov 2, 2022
2b0b7df
Sets authn jackson dependencies to api
DarshitChanpura Nov 3, 2022
554aad6
Modifies authn package to allow deserialization of User object withou…
DarshitChanpura Nov 3, 2022
41470d9
Grants auth codebase to have all permission to javax security to allo…
DarshitChanpura Nov 3, 2022
f22287d
Adds logger statement and fixes spotless errors
DarshitChanpura Nov 3, 2022
8079479
Modifies authenticate method to continue with flow even if authentica…
DarshitChanpura Nov 3, 2022
044317b
Fixes thirdPartyAudit task failure by update list of ignoredClasses
DarshitChanpura Nov 9, 2022
109ee9b
Merge branch 'feature/identity' into basic-auth-via-internal-idp
DarshitChanpura Nov 15, 2022
cf127a7
Adds fake commit to force re-run GHA runners for a transient error in CI
DarshitChanpura Nov 16, 2022
927be7f
Adds unit and integration tests to verify basic auth functionality
DarshitChanpura Nov 17, 2022
eddd2a2
Adds unit and integration tests to verify basic auth functionality
DarshitChanpura Nov 17, 2022
6b1cf9d
Addresses PR feedback
DarshitChanpura Nov 18, 2022
dbfbcb3
Moves internal package from identity to authn, implements getPrinicpa…
DarshitChanpura Nov 19, 2022
c14ab0c
Fixes precommit failures
DarshitChanpura Nov 22, 2022
1ab5c96
Updates username-password rules to be same as security plugin
DarshitChanpura Nov 30, 2022
4b0aa3a
Updates RestController to reject unauthorized requests and allow requ…
DarshitChanpura Dec 1, 2022
5ac1c09
Renames auth tests to have IT extension since its a rest test case
DarshitChanpura Dec 1, 2022
5faad80
Register authn tests to integTest task
DarshitChanpura Dec 1, 2022
aac8bfd
Updates ReindexFromRemoteWithAuthTests.java to accomodate native auth…
DarshitChanpura Dec 1, 2022
4848e59
Adds test_user used by other ITs in example_users yml
DarshitChanpura Dec 1, 2022
a747867
Sets Threadleakscop to none for reindex remote auth tests
DarshitChanpura Dec 1, 2022
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- [Identity] Prototype Internal IdP ([#4659](https://github.com/opensearch-project/OpenSearch/pull/4659))
- [Identity] Strategy for Delegated Authority using Tokens ([#4826](https://github.com/opensearch-project/OpenSearch/pull/4826))
- [Identity] User operations: create update delete ([#4741](https://github.com/opensearch-project/OpenSearch/pull/4741))
- [Identity] Adds Basic Auth mechanism via Internal IdP ([#4798](https://github.com/opensearch-project/OpenSearch/pull/4798))


### Dependencies
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

package org.opensearch.index.reindex;

import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
import org.apache.lucene.util.SetOnce;
import org.opensearch.OpenSearchSecurityException;
import org.opensearch.OpenSearchStatusException;
Expand Down Expand Up @@ -82,6 +83,7 @@
import static org.opensearch.index.reindex.ReindexTestCase.matcher;
import static org.hamcrest.Matchers.containsString;

@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
public class ReindexFromRemoteWithAuthTests extends OpenSearchSingleNodeTestCase {
private TransportAddress address;

Expand Down Expand Up @@ -144,6 +146,7 @@ public void testReindexSendsHeaders() throws Exception {
ReindexRequestBuilder request = new ReindexRequestBuilder(client(), ReindexAction.INSTANCE).source("source")
.destination("dest")
.setRemoteInfo(newRemoteInfo(null, null, singletonMap(TestFilter.EXAMPLE_HEADER, "doesn't matter")));

OpenSearchStatusException e = expectThrows(OpenSearchStatusException.class, () -> request.get());
assertEquals(RestStatus.BAD_REQUEST, e.status());
assertThat(e.getMessage(), containsString("Hurray! Sent the header!"));
Expand All @@ -164,7 +167,8 @@ public void testReindexWithBadAuthentication() throws Exception {
.destination("dest")
.setRemoteInfo(newRemoteInfo("junk", "auth", emptyMap()));
OpenSearchStatusException e = expectThrows(OpenSearchStatusException.class, () -> request.get());
assertThat(e.getMessage(), containsString("\"reason\":\"Bad Authorization\""));
assertThat(e.getMessage(), containsString("\"error\":\"junk does not exist in internal realm.\"")); // Due to native auth
// implementation
}

/**
Expand Down
6 changes: 5 additions & 1 deletion sandbox/libs/authn/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ dependencies {
implementation "com.fasterxml.jackson.core:jackson-annotations:${versions.jackson}"
implementation "com.fasterxml.jackson.core:jackson-databind:${versions.jackson_databind}"

implementation 'org.apache.shiro:shiro-core:1.9.1'
api 'org.apache.shiro:shiro-core:1.9.1'
Copy link
Contributor

Choose a reason for hiding this comment

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

This will let you access internal dependencies of shiro, is that the intention?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes

Copy link
Member

Choose a reason for hiding this comment

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

I don't think this change is needed, Could you provide the why of switching from implementation to api dependency type?

Copy link
Member Author

Choose a reason for hiding this comment

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

We need to set it as api otherwise it throws error: package org.apache.shiro.authc does not exist inside RestController.java#L38

For core to be able to use Shiro module imported via authn package, shiro package needs to be public (i.e. api). If it is changed to implementation shiro import becomes private and we will have to import it separately in server/build.gradle to use it.

If I change it back to implementation here and add a line server/build.gradle like implementation 'org.apache.shiro:shiro-core:1.9.1' the gradle works fine.

IMO there is no need to import Shiro twice as it is not a big build performance issue in using api vs implementation here.

I found this helpful: https://stackoverflow.com/questions/44413952/gradle-implementation-vs-api-configuration

// Needed for shiro
implementation "org.slf4j:slf4j-api:${versions.slf4j}"
implementation "org.bouncycastle:bcprov-jdk15on:${versions.bouncycastle}"
Expand Down Expand Up @@ -276,3 +276,7 @@ thirdPartyAudit.ignoreMissingClasses(
'org.yaml.snakeyaml.parser.ParserImpl',
'org.yaml.snakeyaml.resolver.Resolver',
)

tasks.register("integTest", Test) {
include '**/*IT.class'
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.identity;
package org.opensearch.authn;

import org.opensearch.authn.AccessToken;
import org.opensearch.authn.tokens.AccessToken;

/**
* Vends out access tokens
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.identity;

import org.opensearch.authn.Subject;
package org.opensearch.authn;

/**
* Authentication management for OpenSearch.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.authn;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.shiro.authc.AuthenticationToken;
import org.apache.shiro.authc.UsernamePasswordToken;
import org.opensearch.authn.tokens.BasicAuthToken;

import java.nio.charset.StandardCharsets;
import java.util.Base64;

/**
* Extracts Shiro's {@link AuthenticationToken} from different types of auth headers
*
* @opensearch.experimental
*/
public class AuthenticationTokenHandler {

private static final Logger logger = LogManager.getLogger(AuthenticationTokenHandler.class);

/**
* Extracts shiro auth token from the given header token
* @param authenticationToken the token from which to extract
* @return the extracted shiro auth token to be used to perform login
*/
public static AuthenticationToken extractShiroAuthToken(org.opensearch.authn.tokens.AuthenticationToken authenticationToken) {
AuthenticationToken authToken = null;

if (authenticationToken instanceof BasicAuthToken) {
authToken = handleBasicAuth((BasicAuthToken) authenticationToken);
}
// TODO: check for other type of HeaderTokens
return authToken;
}

/**
* Returns auth token extracted from basic auth header
* @param token the basic auth token
* @return the extracted auth token
*/
private static AuthenticationToken handleBasicAuth(final BasicAuthToken token) {

final byte[] decodedAuthHeader = Base64.getDecoder().decode(token.getHeaderValue().substring("Basic".length()).trim());
String decodedHeader = new String(decodedAuthHeader, StandardCharsets.UTF_8);

final int firstColonIndex = decodedHeader.indexOf(':');

String username = null;
String password = null;

if (firstColonIndex > 0) {
username = decodedHeader.substring(0, firstColonIndex);

if (decodedHeader.length() - 1 != firstColonIndex) {
password = decodedHeader.substring(firstColonIndex + 1);
} else {
// blank password
password = "";
}
}

if (username == null || password == null) {
logger.warn("Invalid 'Authorization' header, send 401 and 'WWW-Authenticate Basic'");
return null;
}

logger.info("Logging in as: " + username);

return new UsernamePasswordToken(username, password);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.InjectableValues;
import com.fasterxml.jackson.databind.MapperFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.json.JsonMapper;
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;

import java.io.IOException;
Expand All @@ -18,14 +20,18 @@
* @opensearch.experimental
*/
public class DefaultObjectMapper {
public static final ObjectMapper objectMapper = new ObjectMapper();
public static ObjectMapper objectMapper;
public final static ObjectMapper YAML_MAPPER = new ObjectMapper(new YAMLFactory());
private static final ObjectMapper defaulOmittingObjectMapper = new ObjectMapper();

static {
objectMapper = JsonMapper.builder()
.enable(JsonParser.Feature.STRICT_DUPLICATE_DETECTION)
.disable(MapperFeature.CAN_OVERRIDE_ACCESS_MODIFIERS) // to prevent access denied exception by Jackson
.build();

objectMapper.setSerializationInclusion(Include.NON_NULL);
// objectMapper.enable(DeserializationFeature.FAIL_ON_TRAILING_TOKENS);
objectMapper.enable(JsonParser.Feature.STRICT_DUPLICATE_DETECTION);
Copy link
Member Author

Choose a reason for hiding this comment

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

enable/ disable are deprecated and JsonMapper.builder() is suggested to be used for building a mapper

defaulOmittingObjectMapper.setSerializationInclusion(Include.NON_DEFAULT);
defaulOmittingObjectMapper.enable(JsonParser.Feature.STRICT_DUPLICATE_DETECTION);
YAML_MAPPER.enable(JsonParser.Feature.STRICT_DUPLICATE_DETECTION);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public enum Principals {

private final Principal principal;

private Principals(final Principal principal) {
Principals(final Principal principal) {
this.principal = principal;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@

package org.opensearch.authn;

import org.opensearch.authn.tokens.AuthenticationToken;

import java.security.Principal;

/**
* An individual, process, or device that causes information to flow among objects or change to the system state.
*
* Used to authorize activities inside of the OpenSearch ecosystem.
*
* @opensearch.experimental
*/
public interface Subject {
Expand All @@ -22,7 +22,7 @@ public interface Subject {
Principal getPrincipal();

/**
* Authentications from a token
* Authentication check via the token
* throws UnsupportedAuthenticationMethod
* throws InvalidAuthenticationToken
* throws SubjectNotFound
Expand Down
10 changes: 10 additions & 0 deletions sandbox/libs/authn/src/main/java/org/opensearch/authn/User.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,21 @@
import java.util.Collections;
import java.util.Map;

/**
* A non-volatile and immutable object in the storage.
*
* @opensearch.experimental
*/

public class User {

@JsonProperty(value = "primary_principal")
private StringPrincipal primaryPrincipal;

@JsonProperty(value = "hash")
private String bcryptHash;

@JsonProperty(value = "attributes")
private Map<String, String> attributes = Collections.emptyMap();

@JsonProperty(value = "primary_principal")
Expand All @@ -39,10 +47,12 @@ public void setBcryptHash(String bcryptHash) {
this.bcryptHash = bcryptHash;
}

@JsonProperty(value = "attributes")
Copy link
Contributor

Choose a reason for hiding this comment

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

Not really a review comment just curious though, what does this syntax denote?

Copy link
Member Author

Choose a reason for hiding this comment

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

When serializing/deserializing a JSON object from/to a class, the mapper will know which field to map against when it see attributes in JSON object when deserializing and writes it as attributes when serializing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh gotcha. Thank you for clearing that up. Overall this looks great!

public Map<String, String> getAttributes() {
return attributes;
}

@JsonProperty(value = "attributes")
public void setAttributes(Map<String, String> attributes) {
this.attributes = attributes;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/
package org.opensearch.authn.internal;

import org.opensearch.authn.AccessTokenManager;
import org.opensearch.authn.tokens.AccessToken;

/**
* Implementation of access token manager that does not enforce authentication
*
* This class and related classes in this package will not return nulls or fail permissions checks
*
* @opensearch.internal
*/
public class InternalAccessTokenManager implements AccessTokenManager {

@Override
public void expireAllTokens() {
// Tokens cannot be expired
}

@Override
public AccessToken generate() {
return new AccessToken();
}

@Override
public AccessToken refresh(final AccessToken token) {
return new AccessToken();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.authn.internal;

import org.opensearch.authn.AccessTokenManager;
import org.opensearch.authn.AuthenticationManager;
import org.opensearch.authn.realm.InternalRealm;
import org.opensearch.authn.Subject;
import org.apache.shiro.SecurityUtils;
import org.apache.shiro.mgt.DefaultSecurityManager;
import org.apache.shiro.mgt.SecurityManager;

/**
* Implementation of authentication manager that enforces authentication against internal idp
*
* This class and related classes in this package will not return nulls or fail permissions checks
*
* This class manages the subjects loaded via the realm, and provides current subject
* when authenticating the incoming request
* Checkout
* and how the internal Identity system uses auth manager to get current subject to use for authentication
*
* @opensearch.internal
*/
public class InternalAuthenticationManager implements AuthenticationManager {

/**
* Security manager is loaded with default user set,
* and this instantiation uses the default security manager
*/
public InternalAuthenticationManager() {
final SecurityManager securityManager = new DefaultSecurityManager(InternalRealm.INSTANCE);
SecurityUtils.setSecurityManager(securityManager);
}

/**
* Instantiates this Auth manager by setting the custom security Manager that is passed as an argument
* @param securityManager the custom security manager (with realm instantiated in it)
*/
public InternalAuthenticationManager(SecurityManager securityManager) {
SecurityUtils.setSecurityManager(securityManager);
}

@Override
public Subject getSubject() {
return new InternalSubject(SecurityUtils.getSubject());
}

@Override
public AccessTokenManager getAccessTokenManager() {
return null;
}
}
Loading