From 6eeadd685cb957e9fd8fcd8aae2e5928f944e587 Mon Sep 17 00:00:00 2001 From: Sergii Leschenko Date: Thu, 19 Jan 2017 17:20:21 +0200 Subject: [PATCH] Rework EnvironmentContext#getSubject to not return null subject --- .../api/core/rest/DefaultHttpJsonRequest.java | 10 +--------- .../che/api/core/rest/HttpJsonHelper.java | 10 +--------- .../che/commons/env/EnvironmentContext.java | 8 +++++++- .../eclipse/che/commons/subject/Subject.java | 12 ++++++++++++ .../ServerContainerInitializeListener.java | 4 ---- .../che/commons/env/EnvironmentContextTest.java | 17 +++++++++++++++++ .../machine/server/util/RecipeDownloader.java | 6 ++---- .../api/workspace/server/WorkspaceManager.java | 4 ++-- 8 files changed, 42 insertions(+), 29 deletions(-) diff --git a/core/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/DefaultHttpJsonRequest.java b/core/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/DefaultHttpJsonRequest.java index 24674be9aeb..e37d226dcc0 100644 --- a/core/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/DefaultHttpJsonRequest.java +++ b/core/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/DefaultHttpJsonRequest.java @@ -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); @@ -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{" + diff --git a/core/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/HttpJsonHelper.java b/core/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/HttpJsonHelper.java index c6be00e03ce..09c509095a0 100644 --- a/core/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/HttpJsonHelper.java +++ b/core/che-core-api-core/src/main/java/org/eclipse/che/api/core/rest/HttpJsonHelper.java @@ -400,14 +400,6 @@ public List requestArray(Class 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, @@ -422,7 +414,7 @@ public String requestString(int timeout, Object body, Pair... 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. diff --git a/core/che-core-api-core/src/main/java/org/eclipse/che/commons/env/EnvironmentContext.java b/core/che-core-api-core/src/main/java/org/eclipse/che/commons/env/EnvironmentContext.java index 0b83ca277dd..667c02863da 100644 --- a/core/che-core-api-core/src/main/java/org/eclipse/che/commons/env/EnvironmentContext.java +++ b/core/che-core-api-core/src/main/java/org/eclipse/che/commons/env/EnvironmentContext.java @@ -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; } diff --git a/core/che-core-api-core/src/main/java/org/eclipse/che/commons/subject/Subject.java b/core/che-core-api-core/src/main/java/org/eclipse/che/commons/subject/Subject.java index 43924472e99..2225aafb89d 100644 --- a/core/che-core-api-core/src/main/java/org/eclipse/che/commons/subject/Subject.java +++ b/core/che-core-api-core/src/main/java/org/eclipse/che/commons/subject/Subject.java @@ -46,6 +46,11 @@ public String getToken() { return null; } + @Override + public boolean isAnonymous() { + return true; + } + @Override public boolean isTemporary() { return false; @@ -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. */ diff --git a/core/che-core-api-core/src/main/java/org/eclipse/che/everrest/ServerContainerInitializeListener.java b/core/che-core-api-core/src/main/java/org/eclipse/che/everrest/ServerContainerInitializeListener.java index cbdd7b2b84c..eca16312bed 100644 --- a/core/che-core-api-core/src/main/java/org/eclipse/che/everrest/ServerContainerInitializeListener.java +++ b/core/che-core-api-core/src/main/java/org/eclipse/che/everrest/ServerContainerInitializeListener.java @@ -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() { diff --git a/core/che-core-api-core/src/test/java/org/eclipse/che/commons/env/EnvironmentContextTest.java b/core/che-core-api-core/src/test/java/org/eclipse/che/commons/env/EnvironmentContextTest.java index 5e5fa10e23c..75ea9a05797 100644 --- a/core/che-core-api-core/src/test/java/org/eclipse/che/commons/env/EnvironmentContextTest.java +++ b/core/che-core-api-core/src/test/java/org/eclipse/che/commons/env/EnvironmentContextTest.java @@ -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 diff --git a/wsmaster/che-core-api-machine/src/main/java/org/eclipse/che/api/machine/server/util/RecipeDownloader.java b/wsmaster/che-core-api-machine/src/main/java/org/eclipse/che/api/machine/server/util/RecipeDownloader.java index 09997648a45..95d491e883e 100644 --- a/wsmaster/che-core-api-machine/src/main/java/org/eclipse/che/api/machine/server/util/RecipeDownloader.java +++ b/wsmaster/che-core-api-machine/src/main/java/org/eclipse/che/api/machine/server/util/RecipeDownloader.java @@ -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()); } } @@ -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()); } } diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceManager.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceManager.java index 585416bdb49..8e5fc8817d0 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceManager.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceManager.java @@ -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;