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.

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

Rework EnvironmentContext#getSubject method to not return null subject #3813

Merged
merged 1 commit into from
Jan 23, 2017
Merged

Rework EnvironmentContext#getSubject method to not return null subject #3813

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 1 addition & 9 deletions ...che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/DefaultHttpJsonRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ protected DefaultHttpJsonResponse doRequest(int timeout,
UnauthorizedException,
ConflictException,
BadRequestException {
final String authToken = getAuthenticationToken();
final String authToken = EnvironmentContext.getCurrent().getSubject().getToken();
final boolean hasQueryParams = parameters != null && !parameters.isEmpty();
if (hasQueryParams || authToken != null) {
final UriBuilder ub = UriBuilder.fromUri(url);
Expand Down Expand Up @@ -293,14 +293,6 @@ protected DefaultHttpJsonResponse doRequest(int timeout,
}
}

private String getAuthenticationToken() {
final Subject subject = EnvironmentContext.getCurrent().getSubject();
if (subject != null) {
return subject.getToken();
}
return null;
}

@Override
public String toString() {
return "DefaultHttpJsonRequest{" +
Expand Down
10 changes: 1 addition & 9 deletions core/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/HttpJsonHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,14 +400,6 @@ public <DTO> List<DTO> requestArray(Class<DTO> dtoInterface,
return null;
}

private String getAuthenticationToken() {
Subject subject = EnvironmentContext.getCurrent().getSubject();
if (subject != null) {
return subject.getToken();
}
return null;
}

public String requestString(String url,
String method,
Object body,
Expand All @@ -422,7 +414,7 @@ public String requestString(int timeout,
Object body,
Pair<String, ?>... parameters)
throws IOException, ServerException, ForbiddenException, NotFoundException, UnauthorizedException, ConflictException {
final String authToken = getAuthenticationToken();
final String authToken = EnvironmentContext.getCurrent().getSubject().getToken();
if ((parameters != null && parameters.length > 0) || authToken != null) {
final UriBuilder ub = UriBuilder.fromUri(url);
//remove sensitive information from url.
Expand Down
8 changes: 7 additions & 1 deletion core/che-core-api-core/src/main/java/org/eclipse/che/commons/env/EnvironmentContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,16 @@ public EnvironmentContext(EnvironmentContext other) {
setSubject(other.getSubject());
}

/**
* Returns subject or {@link Subject#ANONYMOUS} in case when subject is null.
*/
public Subject getSubject() {
return subject;
return subject == null ? Subject.ANONYMOUS : subject;
}

/**
* Sets subject.
*/
public void setSubject(Subject subject) {
this.subject = subject;
}
Expand Down
12 changes: 12 additions & 0 deletions core/che-core-api-core/src/main/java/org/eclipse/che/commons/subject/Subject.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ public String getToken() {
return null;
}

@Override
public boolean isAnonymous() {
return true;
}

@Override
public boolean isTemporary() {
return false;
Expand Down Expand Up @@ -86,6 +91,13 @@ public boolean isTemporary() {
*/
String getToken();

/**
* Return {@code true} if subject is anonymous, {@code false} if this is a real authenticated subject.
*/
default boolean isAnonymous() {
return false;
}

/**
* @return - true if subject is temporary, false if this is a real persistent subject.
*/
Expand Down
4 changes: 0 additions & 4 deletions ...re-api-core/src/main/java/org/eclipse/che/everrest/ServerContainerInitializeListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,10 +187,6 @@ protected SecurityContext createSecurityContext(final HandshakeRequest req) {
final String authType = "BASIC";
final Subject subject = EnvironmentContext.getCurrent().getSubject();

if (subject == null) {
return new SimpleSecurityContext(isSecure);
}

final Principal principal = new SimplePrincipal(subject.getUserName());
return new SecurityContext() {

Expand Down
17 changes: 17 additions & 0 deletions core/che-core-api-core/src/test/java/org/eclipse/che/commons/env/EnvironmentContextTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,23 @@ public void shouldBeAbleToSetEnvContextInSameThread() {
assertFalse(actualSubject.isTemporary());
}

@Test
public void shouldReturnAnonymousSubjectWhenThereIsNoSubject() {
//given
EnvironmentContext expected = EnvironmentContext.getCurrent();
expected.setSubject(null);

//when
Subject actualSubject = EnvironmentContext.getCurrent().getSubject();

//then
assertEquals(actualSubject.getUserName(), Subject.ANONYMOUS.getUserName());
assertEquals(actualSubject.getUserId(), Subject.ANONYMOUS.getUserId());
assertEquals(actualSubject.getToken(), Subject.ANONYMOUS.getToken());
assertEquals(actualSubject.isTemporary(), Subject.ANONYMOUS.isTemporary());
assertEquals(actualSubject.isAnonymous(), Subject.ANONYMOUS.isAnonymous());
}

@Test(enabled = false)
public void shouldNotBeAbleToSeeContextInOtherThread() {
//given
Expand Down
6 changes: 2 additions & 4 deletions ...e-api-machine/src/main/java/org/eclipse/che/api/machine/server/util/RecipeDownloader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,7 @@ public RecipeImpl getRecipe(MachineConfig machineConfig) throws MachineException
.host(apiEndpoint.getHost())
.port(apiEndpoint.getPort())
.replacePath(apiEndpoint.getPath() + location);
if (EnvironmentContext.getCurrent().getSubject() != null
&& EnvironmentContext.getCurrent().getSubject().getToken() != null) {
if (EnvironmentContext.getCurrent().getSubject().getToken() != null) {
targetUriBuilder.queryParam("token", EnvironmentContext.getCurrent().getSubject().getToken());
}
}
Expand Down Expand Up @@ -114,8 +113,7 @@ public String getRecipe(String location) throws ServerException {
.host(apiEndpoint.getHost())
.port(apiEndpoint.getPort())
.replacePath(apiEndpoint.getPath() + location);
if (EnvironmentContext.getCurrent().getSubject() != null
&& EnvironmentContext.getCurrent().getSubject().getToken() != null) {
if (EnvironmentContext.getCurrent().getSubject().getToken() != null) {
targetUriBuilder.queryParam("token", EnvironmentContext.getCurrent().getSubject().getToken());
}
}
Expand Down
4 changes: 2 additions & 2 deletions ...re-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -798,8 +798,8 @@ private Subject sessionUser() {
}

private String sessionUserNameOr(String nameIfNoUser) {
final Subject subject;
if (EnvironmentContext.getCurrent() != null && (subject = EnvironmentContext.getCurrent().getSubject()) != null) {
final Subject subject = EnvironmentContext.getCurrent().getSubject();
if (!subject.isAnonymous()) {
return subject.getUserName();
}
return nameIfNoUser;
Expand Down