From c0dd1d7133fcbbaaba839e597af8855694824aa3 Mon Sep 17 00:00:00 2001 From: Dragomir Todorov Date: Fri, 14 Dec 2018 17:15:29 +0200 Subject: [PATCH 1/3] NY-6005: Add header handling in PermissionInterceptor --- .../permissions/PermissionsInterceptor.java | 100 +++++++++--------- 1 file changed, 50 insertions(+), 50 deletions(-) diff --git a/src/main/java/biz/nynja/account/permissions/PermissionsInterceptor.java b/src/main/java/biz/nynja/account/permissions/PermissionsInterceptor.java index fd9bc08..e4913f8 100644 --- a/src/main/java/biz/nynja/account/permissions/PermissionsInterceptor.java +++ b/src/main/java/biz/nynja/account/permissions/PermissionsInterceptor.java @@ -12,32 +12,28 @@ import org.lognet.springboot.grpc.GRpcGlobalInterceptor; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import io.grpc.Context; -import io.grpc.Contexts; -import io.grpc.Metadata; -import io.grpc.ServerCall; -import io.grpc.ServerCallHandler; -import io.grpc.ServerInterceptor; -import io.grpc.Status; import com.auth0.jwt.JWT; import com.auth0.jwt.interfaces.Claim; import com.auth0.jwt.interfaces.DecodedJWT; import biz.nynja.account.services.AccountServiceImpl; -import biz.nynja.account.grpc.Role; -import biz.nynja.account.permissions.Permitted; +import io.grpc.Context; +import io.grpc.Metadata; +import io.grpc.ServerCall; +import io.grpc.ServerCallHandler; +import io.grpc.ServerInterceptor; /** * @author Stoyan.Tzenkov - account-service ServerInterceptor. Validates roles for granting permissions to - * account-service endpoints(rpcs). - * General rules: - * - if access token is not present - PERMISSION DENIED; - * - if no roles found in the access token - PERMISSION DENIED; - * - if rpc not found in the account-service class - PERMISSION DENIED; - * - if no Permitted annotation found for the rpc method - PERMISSION DENIED; - * - if rpc has either @Permitted(role = RoleConstants.ANY) - PERMISSION GRANTED; - * - if rpc has an annotation @PerformPermissionCheck - @Permitted annotations are checked and an additional check is performed in the rpc; - * - if no role from the request matches any Permitted annotation for the rpc - PERMISSION DENIED + * account-service endpoints(rpcs). General rules: - if access token is not present - PERMISSION DENIED; - if no + * roles found in the access token - PERMISSION DENIED; - if rpc not found in the account-service class - + * PERMISSION DENIED; - if no Permitted annotation found for the rpc method - PERMISSION DENIED; - if rpc has + * either @Permitted(role = RoleConstants.ANY) - PERMISSION GRANTED; - if rpc has an + * annotation @PerformPermissionCheck - @Permitted annotations are checked and an additional check is performed + * in the rpc; - if no role from the request matches any Permitted annotation for the rpc - PERMISSION DENIED + * + * Expected metadata is "Authorization" : "Bearer --accessTokenValue" so we can skip validation as istio won't + * allow this request through */ @GRpcGlobalInterceptor @@ -46,7 +42,7 @@ public class PermissionsInterceptor implements ServerInterceptor { private static final Logger logger = LoggerFactory.getLogger(PermissionsInterceptor.class); private static final Class SERVICE_CLASS = AccountServiceImpl.class; - public static final Metadata.Key ACCESS_TOKEN_METADATA = Metadata.Key.of("accessToken", + public static final Metadata.Key ACCESS_TOKEN_METADATA = Metadata.Key.of("Authorization", ASCII_STRING_MARSHALLER); public static final Context.Key ACCESS_TOKEN_CTX = Context.key("accessToken"); @@ -61,37 +57,41 @@ public class PermissionsInterceptor implements ServerInterceptor { // when Istio starts sending an access token with each and every request return next.startCall(call, headers); -// String rpc = getRpcName(call); -// -// String accessToken = headers.get(ACCESS_TOKEN_METADATA); -// -// boolean permitted = false; -// Context ctx = null; -// String[] requestingRoles = null; -// -// if (accessToken != null) { -// ctx = Context.current().withValue(ACCESS_TOKEN_CTX, accessToken); -// requestingRoles = getRolesFromAccessToken(accessToken, rpc); -// -// if (requestingRoles != null) { -// Method method = getMethod(rpc); -// -// if (method != null) { -// Permitted[] permittedRoles = method.getAnnotationsByType(Permitted.class); -// permitted = checkPermissions(requestingRoles, permittedRoles); -// } -// } -// } -// -// if (permitted) { -// logger.info("Permission granted to rpc {}.", rpc); -// return Contexts.interceptCall(ctx, call, headers, next); -// } else { -// logger.error("Permission denied for rpc {}, roles {}.", rpc, requestingRoles); -// call.close(Status.PERMISSION_DENIED.withDescription("An unauthorized call was made to " + rpc + "."), -// headers); -// return NOOP_LISTENER; -// } + /** + * accessToken key is now Authorization with format Bearer accessTokenValue + */ + // new String accessToken = (headers.get(ACCESS_TOKEN_METADATA).split(" "))[1]; + // old String accessToken = headers.get(ACCESS_TOKEN_METADATA); + + // String rpc = getRpcName(call); + // + // boolean permitted = false; + // Context ctx = null; + // String[] requestingRoles = null; + // + // if (accessToken != null) { + // ctx = Context.current().withValue(ACCESS_TOKEN_CTX, accessToken); + // requestingRoles = getRolesFromAccessToken(accessToken, rpc); + // + // if (requestingRoles != null) { + // Method method = getMethod(rpc); + // + // if (method != null) { + // Permitted[] permittedRoles = method.getAnnotationsByType(Permitted.class); + // permitted = checkPermissions(requestingRoles, permittedRoles); + // } + // } + // } + // + // if (permitted) { + // logger.info("Permission granted to rpc {}.", rpc); + // return Contexts.interceptCall(ctx, call, headers, next); + // } else { + // logger.error("Permission denied for rpc {}, roles {}.", rpc, requestingRoles); + // call.close(Status.PERMISSION_DENIED.withDescription("An unauthorized call was made to " + rpc + "."), + // headers); + // return NOOP_LISTENER; + // } } private String getRpcName(ServerCall call) { -- GitLab From 07ab990d5ec19bbff79f1a865a38871836c75865 Mon Sep 17 00:00:00 2001 From: Dragomir Todorov Date: Mon, 17 Dec 2018 15:52:58 +0200 Subject: [PATCH 2/3] NY-6005: Added logic to split the authentication header --- .../account/permissions/PermissionsInterceptor.java | 11 ++++------- .../nynja/account/permissions/RoleConstants.java | 4 ++-- .../nynja/account/services/AccountServiceImpl.java | 13 ++++++++----- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/main/java/biz/nynja/account/permissions/PermissionsInterceptor.java b/src/main/java/biz/nynja/account/permissions/PermissionsInterceptor.java index e4913f8..639a610 100644 --- a/src/main/java/biz/nynja/account/permissions/PermissionsInterceptor.java +++ b/src/main/java/biz/nynja/account/permissions/PermissionsInterceptor.java @@ -32,8 +32,6 @@ import io.grpc.ServerInterceptor; * annotation @PerformPermissionCheck - @Permitted annotations are checked and an additional check is performed * in the rpc; - if no role from the request matches any Permitted annotation for the rpc - PERMISSION DENIED * - * Expected metadata is "Authorization" : "Bearer --accessTokenValue" so we can skip validation as istio won't - * allow this request through */ @GRpcGlobalInterceptor @@ -57,12 +55,11 @@ public class PermissionsInterceptor implements ServerInterceptor { // when Istio starts sending an access token with each and every request return next.startCall(call, headers); - /** - * accessToken key is now Authorization with format Bearer accessTokenValue + /* + * Expected metadata is "Authorization" : "Bearer --accessTokenValue--" so we can skip validation as istio won't + * allow this request through */ - // new String accessToken = (headers.get(ACCESS_TOKEN_METADATA).split(" "))[1]; - // old String accessToken = headers.get(ACCESS_TOKEN_METADATA); - + // String accessToken = (headers.get(ACCESS_TOKEN_METADATA).split(" "))[1]; // String rpc = getRpcName(call); // // boolean permitted = false; diff --git a/src/main/java/biz/nynja/account/permissions/RoleConstants.java b/src/main/java/biz/nynja/account/permissions/RoleConstants.java index 8ca5403..4a9b8d4 100644 --- a/src/main/java/biz/nynja/account/permissions/RoleConstants.java +++ b/src/main/java/biz/nynja/account/permissions/RoleConstants.java @@ -7,10 +7,10 @@ package biz.nynja.account.permissions; * changes need to be made to the Role enum in account.proto accordingly */ public class RoleConstants { - + public static final String ADMIN = "ADMIN"; public static final String USER = "USER"; public static final String ANY = "ANY"; - public static final String AUTH_SERVICE = "AUTH-SERVICE"; + public static final String AUTH_SERVICE = "AUTH_SERVICE"; } diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 0788290..3c0666b 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -3,6 +3,11 @@ */ package biz.nynja.account.services; +import static biz.nynja.account.validation.Validators.account; +import static biz.nynja.account.validation.Validators.authenticationProvider; +import static biz.nynja.account.validation.Validators.phoneValidator; +import static biz.nynja.account.validation.Validators.util; + import java.util.List; import java.util.Optional; import java.util.UUID; @@ -11,6 +16,7 @@ import org.apache.commons.lang3.tuple.ImmutablePair; import org.lognet.springboot.grpc.GRpcService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; + import biz.nynja.account.grpc.AccountByAccountIdRequest; import biz.nynja.account.grpc.AccountResponse; import biz.nynja.account.grpc.AccountServiceGrpc; @@ -54,6 +60,7 @@ import biz.nynja.account.models.ProfileByAuthenticationProvider; import biz.nynja.account.permissions.PerformPermissionCheck; import biz.nynja.account.permissions.PermissionsValidator; import biz.nynja.account.permissions.Permitted; +import biz.nynja.account.permissions.RoleConstants; import biz.nynja.account.phone.PhoneNumberNormalizer; import biz.nynja.account.repositories.AccountByProfileIdRepository; import biz.nynja.account.repositories.AccountByQrCodeRepository; @@ -67,13 +74,8 @@ import biz.nynja.account.services.decomposition.IncorrectAccountCountException; import biz.nynja.account.services.decomposition.ProfileProvider; import biz.nynja.account.validation.Validation; import biz.nynja.account.validation.ValidationError; -import biz.nynja.account.permissions.RoleConstants; import io.grpc.stub.StreamObserver; -import static biz.nynja.account.validation.Validators.*; -import static biz.nynja.account.validation.Validators.account; -import static biz.nynja.account.validation.Validators.util; - /** * gRPC Account service implementation.
* The service extends the protobuf generated class and overrides the needed methods. It also saves/retrieves the @@ -1088,6 +1090,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas @PerformPermissionCheck @Permitted(role = RoleConstants.ADMIN) @Permitted(role = RoleConstants.USER) + @Permitted(role = RoleConstants.AUTH_SERVICE) public void getAccountByLoginOption(AuthenticationProviderRequest request, StreamObserver responseObserver) { logger.info("Getting account by login option: {}:{} ", request.getAuthenticationType(), -- GitLab From 7017fe954fd4bc5618a21eb2366e4bab4cde62eb Mon Sep 17 00:00:00 2001 From: Dragomir Todorov Date: Tue, 18 Dec 2018 12:25:48 +0200 Subject: [PATCH 3/3] NY-6005: Fixed comment aligment --- .../permissions/PermissionsInterceptor.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/main/java/biz/nynja/account/permissions/PermissionsInterceptor.java b/src/main/java/biz/nynja/account/permissions/PermissionsInterceptor.java index 639a610..1fc1d13 100644 --- a/src/main/java/biz/nynja/account/permissions/PermissionsInterceptor.java +++ b/src/main/java/biz/nynja/account/permissions/PermissionsInterceptor.java @@ -25,13 +25,15 @@ import io.grpc.ServerInterceptor; /** * @author Stoyan.Tzenkov - account-service ServerInterceptor. Validates roles for granting permissions to - * account-service endpoints(rpcs). General rules: - if access token is not present - PERMISSION DENIED; - if no - * roles found in the access token - PERMISSION DENIED; - if rpc not found in the account-service class - - * PERMISSION DENIED; - if no Permitted annotation found for the rpc method - PERMISSION DENIED; - if rpc has - * either @Permitted(role = RoleConstants.ANY) - PERMISSION GRANTED; - if rpc has an - * annotation @PerformPermissionCheck - @Permitted annotations are checked and an additional check is performed - * in the rpc; - if no role from the request matches any Permitted annotation for the rpc - PERMISSION DENIED - * + * account-service endpoints(rpcs). + * General rules: + * - if access token is not present - PERMISSION DENIED; + * - if no roles found in the access token - PERMISSION DENIED; + * - if rpc not found in the account-service class - PERMISSION DENIED; + * - if no Permitted annotation found for the rpc method - PERMISSION DENIED; + * - if rpc has either @Permitted(role = RoleConstants.ANY) - PERMISSION GRANTED; + * - if rpc has an annotation @PerformPermissionCheck - @Permitted annotations are checked and an additional check is performed in the rpc; + * - if no role from the request matches any Permitted annotation for the rpc - PERMISSION DENIED */ @GRpcGlobalInterceptor -- GitLab