Skip to content

Commit

Permalink
Improved behaviour when to return 401 or 403
Browse files Browse the repository at this point in the history
  • Loading branch information
@hylkevds
hylkevds committed Oct 4, 2024
1 parent c253ca6 commit 008a18d
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 22 deletions.
15 changes: 12 additions & 3 deletions ...th.Basic/src/main/java/de/fraunhofer/iosb/ilt/frostserver/auth/basic/BasicAuthFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ private boolean requireRole(String roleName, UserData userData, HttpServletRespo

if (!userData.roles.contains(roleName)) {
LOGGER.debug("Rejecting request: User {} does not have role {}.", userData.userName, roleName);
throwAuthRequired(response);
throwInsufficientRights(response);
return false;
}
LOGGER.debug("Accepting request: User {} has role {}.", userData.userName, roleName);
Expand All @@ -179,7 +179,7 @@ public void doFilter(ServletRequest req, ServletResponse resp, FilterChain chain
} catch (IllegalArgumentException exc) {
LOGGER.debug("Rejecting request: Unknown method: {}.", request.getMethod());
LOGGER.trace("", exc);
throwAuthRequired(response);
throwInsufficientRights(response);
return;
}

Expand All @@ -199,7 +199,7 @@ public void doFilter(ServletRequest req, ServletResponse resp, FilterChain chain
AuthChecker checker = methodCheckers.get(method);
if (checker == null) {
LOGGER.debug("Rejecting request: No checker for method: {}.", request.getMethod());
throwAuthRequired(response);
throwInsufficientRights(response);
return;
}

Expand All @@ -223,6 +223,15 @@ private void throwAuthRequired(HttpServletResponse response) {
}
}

private void throwInsufficientRights(HttpServletResponse response) {
response.addHeader(AUTHORIZATION_REQUIRED_HEADER, authHeaderValue);
try {
response.sendError(403);
} catch (IOException exc) {
LOGGER.error("Exception sending back error.", exc);
}
}

private static String getInitParamWithDefault(FilterConfig filterConfig, String paramName, Class<? extends ConfigDefaults> defaultsProvider) {
return getInitParamWithDefault(filterConfig, paramName, ConfigUtils.getDefaultValue(defaultsProvider, paramName));
}
Expand Down
6 changes: 5 additions & 1 deletion ...ycloak/src/main/java/de/fraunhofer/iosb/ilt/frostserver/auth/keycloak/KeycloakFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,10 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
LOGGER.debug("User has correct role.");
chain.doFilter(new RequestWrapper(httpRequest, pe), response);
return;
} else {
LOGGER.debug("User is not allowed.");
throwHttpError(403, httpResponse);
return;
}
}
}
Expand All @@ -246,7 +250,7 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
}
}
LOGGER.debug("User is not allowed.");
throwHttpError(403, httpResponse);
throwHttpError(401, httpResponse);
}

private KeycloakAccount findKeycloakAccount(HttpServletRequest httpRequest) {
Expand Down
11 changes: 8 additions & 3 deletions ...P.Common/src/main/java/de/fraunhofer/iosb/ilt/frostserver/http/common/DatabaseStatus.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,14 @@ protected void processGetRequest(HttpServletRequest request, HttpServletResponse
CoreSettings coreSettings = (CoreSettings) request.getServletContext().getAttribute(TAG_CORE_SETTINGS);
String authProviderClassName = coreSettings.getAuthSettings().get(CoreSettings.TAG_AUTH_PROVIDER, CoreSettings.class);
PrincipalExtended userPrincipal = PrincipalExtended.fromPrincipal(request.getUserPrincipal());
if (!isNullOrEmpty(authProviderClassName) && !userPrincipal.isAdmin()) {
response.sendError(403);
return;
if (!isNullOrEmpty(authProviderClassName)) {
if (userPrincipal == PrincipalExtended.ANONYMOUS_PRINCIPAL) {
response.sendError(401);
return;
} else if (!userPrincipal.isAdmin()) {
response.sendError(403);
return;
}
}
PersistenceManagerFactory.init(coreSettings);

Expand Down
28 changes: 14 additions & 14 deletions ...ver.Tests/src/test/java/de/fraunhofer/iosb/ilt/statests/f01auth/FineGrainedAuthTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,18 +305,18 @@ private String postBatch(String body) {
void test_01_UpdateDb() throws IOException {
LOGGER.info(" test_01_UpdateDb");
ath.getDatabaseStatus(ADMIN, serviceAdmin, HTTP_CODE_200_OK);
ath.getDatabaseStatus(WRITE, serviceWrite, HTTP_CODE_401_UNAUTHORIZED, HTTP_CODE_403_FORBIDDEN);
ath.getDatabaseStatus(READ, serviceRead, HTTP_CODE_401_UNAUTHORIZED, HTTP_CODE_403_FORBIDDEN);
ath.getDatabaseStatusIndirect(serviceAnon, HTTP_CODE_401_UNAUTHORIZED, HTTP_CODE_403_FORBIDDEN);
ath.getDatabaseStatus(ANONYMOUS, serviceAnon, HTTP_CODE_401_UNAUTHORIZED, HTTP_CODE_403_FORBIDDEN);
ath.getDatabaseStatusIndirect(serviceAdminProject1, HTTP_CODE_401_UNAUTHORIZED, HTTP_CODE_403_FORBIDDEN);
ath.getDatabaseStatus(ADMIN_P1, serviceAdminProject1, HTTP_CODE_401_UNAUTHORIZED, HTTP_CODE_403_FORBIDDEN);
ath.getDatabaseStatusIndirect(serviceAdminProject2, HTTP_CODE_401_UNAUTHORIZED, HTTP_CODE_403_FORBIDDEN);
ath.getDatabaseStatus(ADMIN_P2, serviceAdminProject2, HTTP_CODE_401_UNAUTHORIZED, HTTP_CODE_403_FORBIDDEN);
ath.getDatabaseStatusIndirect(serviceObsCreaterProject1, HTTP_CODE_401_UNAUTHORIZED, HTTP_CODE_403_FORBIDDEN);
ath.getDatabaseStatus(OBS_CREATE_P1, serviceObsCreaterProject1, HTTP_CODE_401_UNAUTHORIZED, HTTP_CODE_403_FORBIDDEN);
ath.getDatabaseStatusIndirect(serviceObsCreaterProject2, HTTP_CODE_401_UNAUTHORIZED, HTTP_CODE_403_FORBIDDEN);
ath.getDatabaseStatus(OBS_CREATE_P2, serviceObsCreaterProject2, HTTP_CODE_401_UNAUTHORIZED, HTTP_CODE_403_FORBIDDEN);
ath.getDatabaseStatus(WRITE, serviceWrite, HTTP_CODE_403_FORBIDDEN);
ath.getDatabaseStatus(READ, serviceRead, HTTP_CODE_403_FORBIDDEN);
ath.getDatabaseStatusIndirect(serviceAnon, HTTP_CODE_401_UNAUTHORIZED);
ath.getDatabaseStatus(ANONYMOUS, serviceAnon, HTTP_CODE_401_UNAUTHORIZED);
ath.getDatabaseStatusIndirect(serviceAdminProject1, HTTP_CODE_403_FORBIDDEN);
ath.getDatabaseStatus(ADMIN_P1, serviceAdminProject1, HTTP_CODE_403_FORBIDDEN);
ath.getDatabaseStatusIndirect(serviceAdminProject2, HTTP_CODE_403_FORBIDDEN);
ath.getDatabaseStatus(ADMIN_P2, serviceAdminProject2, HTTP_CODE_403_FORBIDDEN);
ath.getDatabaseStatusIndirect(serviceObsCreaterProject1, HTTP_CODE_403_FORBIDDEN);
ath.getDatabaseStatus(OBS_CREATE_P1, serviceObsCreaterProject1, HTTP_CODE_403_FORBIDDEN);
ath.getDatabaseStatusIndirect(serviceObsCreaterProject2, HTTP_CODE_403_FORBIDDEN);
ath.getDatabaseStatus(OBS_CREATE_P2, serviceObsCreaterProject2, HTTP_CODE_403_FORBIDDEN);
}

@Test
Expand All @@ -325,7 +325,7 @@ void test_02a_ReadProjects() {
testFilterResults(ADMIN, serviceAdmin, mdlUsers.etProject, "", PROJECTS);
testFilterResults(WRITE, serviceWrite, mdlUsers.etProject, "", PROJECTS);
testFilterResults(READ, serviceRead, mdlUsers.etProject, "", PROJECTS);
filterForException(ANONYMOUS, serviceAnon, mdlUsers.etProject, "", H401, H403);
filterForException(ANONYMOUS, serviceAnon, mdlUsers.etProject, "", H401);
testFilterResults(ADMIN_P1, serviceAdminProject1, mdlUsers.etProject, "", PROJECTS);
testFilterResults(ADMIN_P2, serviceAdminProject2, mdlUsers.etProject, "", PROJECTS);
testFilterResults(OBS_CREATE_P1, serviceObsCreaterProject1, mdlUsers.etProject, "", PROJECTS);
Expand All @@ -339,7 +339,7 @@ void test_02b_CreateProject() {

createForOk(WRITE, serviceWrite, creator, serviceAdmin.dao(mdlUsers.etProject), PROJECTS);
createForFail(READ, serviceRead, creator, serviceAdmin.dao(mdlUsers.etProject), PROJECTS, H403);
createForFail(ANONYMOUS, serviceAnon, creator, serviceAdmin.dao(mdlUsers.etProject), PROJECTS, H401, H403);
createForFail(ANONYMOUS, serviceAnon, creator, serviceAdmin.dao(mdlUsers.etProject), PROJECTS, H401);
createForFail(ADMIN_P1, serviceAdminProject1, creator, serviceAdmin.dao(mdlUsers.etProject), PROJECTS, H403);
createForFail(ADMIN_P2, serviceAdminProject2, creator, serviceAdmin.dao(mdlUsers.etProject), PROJECTS, H403);
createForFail(OBS_CREATE_P1, serviceObsCreaterProject1, creator, serviceAdmin.dao(mdlUsers.etProject), PROJECTS, H403);
Expand Down
3 changes: 2 additions & 1 deletion FROST-Server.Tests/src/test/java/de/fraunhofer/iosb/ilt/statests/f01auth/KeyCloakTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package de.fraunhofer.iosb.ilt.statests.f01auth;

import static de.fraunhofer.iosb.ilt.statests.TestSuite.KEY_DB_NAME;
import static de.fraunhofer.iosb.ilt.statests.f01auth.AuthTestHelper.HTTP_CODE_401_UNAUTHORIZED;
import static de.fraunhofer.iosb.ilt.statests.util.EntityUtils.filterForException;
import static de.fraunhofer.iosb.ilt.statests.util.EntityUtils.testFilterResults;

Expand Down Expand Up @@ -121,7 +122,7 @@ protected void setUpVersion() {
void test_100_ReadUser() {
LOGGER.info(" test_100_ReadUser");
testFilterResults(ADMIN, serviceAdmin, mdlUsers.etUser, "", USERS);
filterForException(ANONYMOUS, serviceAnon, mdlUsers.etUser, "", AuthTestHelper.HTTP_CODE_403_FORBIDDEN);
filterForException(ANONYMOUS, serviceAnon, mdlUsers.etUser, "", HTTP_CODE_401_UNAUTHORIZED);
}

@Override
Expand Down

0 comments on commit 008a18d

Please sign in to comment.