Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
ashera96 committed Nov 3, 2023
1 parent bb5a8c8 commit f80e6a8
Show file tree
Hide file tree
Showing 10 changed files with 22 additions and 35 deletions.
2 changes: 1 addition & 1 deletion adapter/internal/operator/synchronizer/data_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func (ods *OperatorDataStore) processAPIState(apiNamespacedName types.Namespaced
}
}

if (cachedAPI.SubscriptionValidation != apiState.SubscriptionValidation) {
if cachedAPI.SubscriptionValidation != apiState.SubscriptionValidation {
cachedAPI.SubscriptionValidation = apiState.SubscriptionValidation
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ spec:
organization: carbon.super
api:
name: AppSubTestAPI
version: ^1\.|^v1$
version: ^1.*|^v1$
6 changes: 1 addition & 5 deletions common-controller/internal/operator/constant/constant.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,5 @@ const (

// Application authentication types
const (
OAuth2 = "OAuth2"
MTLS = "mTLS"
BasicAuth = "basicAuth"
APIKey = "apiKey"
IPRange = "IPRange"
OAuth2 = "OAuth2"
)
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func marshalApplicationKeyMapping(applicationList []cpv1alpha2.Application) *sub
for _, env := range oauth2SecurityScheme.Environments {
appIdentifier := &subscription.ApplicationKeyMapping{
ApplicationUUID: appInternal.Name,
SecurityScheme: "OAuth2",
SecurityScheme: constants.OAuth2,
ApplicationIdentifier: env.AppID,
KeyType: env.KeyType,
EnvID: env.EnvID,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,7 @@ public static class JwtTokenConstants {
public static final String APPLICATION_NAME = "name";
public static final String APPLICATION_TIER = "tier";
public static final String APPLICATION_OWNER = "owner";
public static final String KEY_TYPE = "keyType";
public static final String CLIENT_ID = "clientId";
public static final String SUBSCRIPTION_TIER = "subscriptionTier";
public static final String SUBSCRIPTION_ORGANIZATION = "organization";
public static final String SUBSCRIBER_TENANT_DOMAIN = "subscriberTenantDomain";
public static final String TIER_INFO = "tierInfo";
public static final String STOP_ON_QUOTA_REACH = "stopOnQuotaReach";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public class APISecurityConstants {
public static final String API_SUBSCRIPTION_BLOCKED_DESCRIPTION = "API Subscription is blocked";

public static final int API_AUTH_FORBIDDEN = 900908;
public static final String API_AUTH_FORBIDDEN_MESSAGE = "Resource forbidden ";
public static final String API_AUTH_FORBIDDEN_MESSAGE = "Resource forbidden";

public static final int SUBSCRIPTION_INACTIVE = 900909;
public static final String SUBSCRIPTION_INACTIVE_MESSAGE = "The subscription to the API is inactive";
Expand All @@ -70,9 +70,6 @@ public class APISecurityConstants {
public static final int API_AUTH_MISSING_OPEN_API_DEF = 900911;
public static final String API_AUTH_MISSING_OPEN_API_DEF_ERROR_MESSAGE = "Internal Server Error";

public static final int SUBSCRIPTION_NOT_FOUND = 900912;
public static final String SUBSCRIPTION_NOT_FOUND_MESSAGE = "Subscription validation failed";

// TODO: (renuka) check error codes with APIM: https://github.com/wso2/wso2-synapse/pull/1899/files#r809710868
public static final int OPA_AUTH_FORBIDDEN = 901101;
public static final String OPA_AUTH_FORBIDDEN_MESSAGE = "Forbidden";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,14 +184,15 @@ public static void validateSubscriptionUsingConsumerKey(APIKeyValidationInfoDTO
}
} else {
log.error("Subscription data store is null");
throw new APISecurityException(APIConstants.StatusCodes.UNAUTHENTICATED.getCode(),
APISecurityConstants.API_AUTH_GENERAL_ERROR,
APISecurityConstants.API_AUTH_GENERAL_ERROR_MESSAGE);
}
// If the execution reaches this point, it means that the subscription validation has failed.
if (log.isDebugEnabled()) {
log.debug("User is NOT authorized to access the API. Subscription validation failed for consumer key : "
+ consumerKey);
}
log.error("User is NOT authorized to access the API. Subscription validation failed for consumer key : "
+ consumerKey);
throw new APISecurityException(APIConstants.StatusCodes.UNAUTHORIZED.getCode(),
APISecurityConstants.SUBSCRIPTION_NOT_FOUND, APISecurityConstants.SUBSCRIPTION_NOT_FOUND_MESSAGE);
APISecurityConstants.API_AUTH_FORBIDDEN, APISecurityConstants.API_AUTH_FORBIDDEN_MESSAGE);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,10 @@ public AuthenticationContext authenticate(RequestContext requestContext) throws
consumerKey, envType, APIConstants.API_SECURITY_OAUTH2, organization,
splitToken);
} else {
log.error("Error while extracting consumer key from JWT token claim set");
throw new APISecurityException(APIConstants.StatusCodes.UNAUTHORIZED.getCode(),
APISecurityConstants.SUBSCRIPTION_NOT_FOUND,
APISecurityConstants.SUBSCRIPTION_NOT_FOUND_MESSAGE);
log.error("Error while extracting consumer key from token");
throw new APISecurityException(APIConstants.StatusCodes.UNAUTHENTICATED.getCode(),
APISecurityConstants.API_AUTH_INVALID_CREDENTIALS,
"Invalid JWT token. Error while extracting consumer key from token");
}
} else {
// In this case, the application related properties are populated so that analytics
Expand Down Expand Up @@ -350,19 +350,16 @@ private void validateSubscriptionUsingConsumerKey(APIKeyValidationInfoDTO valida

if (validationInfo.isAuthorized()) {
if (log.isDebugEnabled()) {
log.debug(
"User is subscribed to the API: " + name + ", " + "version: " + version + ". Token:" + " " + FilterUtils.getMaskedToken(
splitToken[0]));
log.debug("User is subscribed to the API: " + name + ", " + "version: " + version + ". Token:" + " " +
FilterUtils.getMaskedToken(splitToken[0]));
}
} else {
if (log.isDebugEnabled()) {
log.debug("User is not subscribed to access the API: " + name + ", version: " + version + ". " +
"Token: " + FilterUtils.getMaskedToken(splitToken[0]));
}
log.error("User is not subscribed to access the API.");
throw new APISecurityException(APIConstants.StatusCodes.UNAUTHORIZED.getCode(),
APISecurityConstants.SUBSCRIPTION_NOT_FOUND,
APISecurityConstants.SUBSCRIPTION_NOT_FOUND_MESSAGE);
APISecurityConstants.API_AUTH_FORBIDDEN, APISecurityConstants.API_AUTH_FORBIDDEN_MESSAGE);
}
}

Expand Down
8 changes: 3 additions & 5 deletions helm-charts/crds/cp.wso2.com_subscriptions.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,11 @@ spec:
properties:
name:
type: string
versions:
items:
type: string
type: array
version:
type: string
required:
- name
- versions
- version
type: object
organization:
type: string
Expand Down
1 change: 1 addition & 0 deletions helm-charts/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ wso2:
usernameClaim: "sub"
organizationClaim: "organization"
groupsClaim: "groups"
consumerKeyClaim: "clientId"
# organizationResolver: "controlPlane" # controlplane,none
# tls:
# secretName: "wso2apk-idp-certificates"
Expand Down

0 comments on commit f80e6a8

Please sign in to comment.