Api application registration & token generation restructuring #536

Open
rajitha wants to merge 2 commits from rajitha/device-mgt-core:auth-restructure into master
Collaborator

Purpose

## Purpose * Contains improvements relates to https://roadmap.entgra.net/issues/12139
rajitha added 2 commits 4 weeks ago
rajitha added 1 commit 4 weeks ago
rajitha force-pushed auth-restructure from 51777d6714 to 1eb2d23adf 3 weeks ago
rajitha force-pushed auth-restructure from 1eb2d23adf to 40a5b00f80 3 weeks ago
rajitha changed title from WIP: DCR and Token generation process restructuring to Api application registration & token generation restructuring 3 weeks ago
charithag requested changes 2 weeks ago
String msg = "Invalid tenant domain : " + tenantDomain;
return Response.status(Response.Status.NOT_ACCEPTABLE).entity(msg).build();
}
Owner
    @Path("register/tenants")
    @POST
    public Response register(@QueryParam("tenantDomain") String tenantDomain,
                             @QueryParam("applicationName") String applicationName) {

How this endpoint secured? if not properly secured it is possible to obtain client credentials for given tenant using this API.

```java @Path("register/tenants") @POST public Response register(@QueryParam("tenantDomain") String tenantDomain, @QueryParam("applicationName") String applicationName) { ``` How this endpoint secured? if not properly secured it is possible to obtain client credentials for given tenant using this API.
ApiApplicationConstants.DEFAULT_TOKEN_TYPE, username, false,
ApiApplicationConstants.DEFAULT_VALIDITY_PERIOD, PrivilegedCarbonContext.getThreadLocalCarbonContext().getUserRealm()
.getRealmConfiguration().getAdminPassword(), null, null, null, false);
ApiApplicationKey apiApplicationKey =
Owner

Can't we cache the apiApplicationKey for future use?

Can't we cache the apiApplicationKey for future use?
if ((registrationProfile.getTags() != null && registrationProfile.getTags().length != 0)) {
if (!APIUtil.getAllowedApisTags().containsAll(Arrays.asList(registrationProfile.getTags()))) {
return Response.status(Response.Status.NOT_ACCEPTABLE).entity("APIs(Tags) are not allowed to this user."
if (!new HashSet<>(APIUtil.getAllowedApisTags()).containsAll(Arrays.asList(registrationProfile.getTags()))) {
Owner
    @Path("register")
    @POST
    public Response register(RegistrationProfile registrationProfile) {

How this endpoint secured? if not properly secured it is possible to obtain client credentials for given registration profile using this API.

```java @Path("register") @POST public Response register(RegistrationProfile registrationProfile) { ``` How this endpoint secured? if not properly secured it is possible to obtain client credentials for given registration profile using this API.
ApiApplicationKey apiApplicationKey =
apiManagementProviderService.registerApiApplication(idnAuthenticationProfile,
apiApplicationProfile);
return Response.status(Response.Status.CREATED).entity(apiApplicationKey).build();
Owner

Can't we cache the apiApplicationKey for future use?

Can't we cache the apiApplicationKey for future use?
@Provider
@Produces(APPLICATION_JSON)
@Consumes(APPLICATION_JSON)
public class GsonMessageBodyHandler implements MessageBodyWriter<Object>, MessageBodyReader<Object> {
Owner

I've seen this GsonMessageBodyHandler code duplicate over all the WARs. Can't we just have a common library with similar stuffs?

I've seen this GsonMessageBodyHandler code duplicate over all the WARs. Can't we just have a common library with similar stuffs?
public class GsonMessageBodyHandler implements MessageBodyWriter<Object>, MessageBodyReader<Object> {
private Gson gson;
private static final String UTF_8 = "UTF-8";
Owner

Do not define constants for existing constants. Use constant provided by framework.

import java.nio.charset.StandardCharsets;

...

StandardCharsets.UTF_8.name();
Do not define constants for existing constants. Use constant provided by framework. ```java import java.nio.charset.StandardCharsets; ... StandardCharsets.UTF_8.name(); ```
<context-param>
<param-name>doAuthentication</param-name>
<param-value>true</param-value>
<param-value>false</param-value>
Owner
This makes a serious security issue. [https://repository.entgra.net/community/device-mgt-core/pulls/536/files#issuecomment-20531](https://repository.entgra.net/community/device-mgt-core/pulls/536/files#issuecomment-20531) [https://repository.entgra.net/community/device-mgt-core/pulls/536/files#issuecomment-20529](https://repository.entgra.net/community/device-mgt-core/pulls/536/files#issuecomment-20529)
<context-param>
<param-name>basicAuth</param-name>
<param-value>true</param-value>
<param-value>false</param-value>
Owner
This makes a serious security issue. [https://repository.entgra.net/community/device-mgt-core/pulls/536/files#issuecomment-20531](https://repository.entgra.net/community/device-mgt-core/pulls/536/files#issuecomment-20531) [https://repository.entgra.net/community/device-mgt-core/pulls/536/files#issuecomment-20529](https://repository.entgra.net/community/device-mgt-core/pulls/536/files#issuecomment-20529)
Poster
Collaborator

Previous dcr endpoints were unsecure, thought there are uses of that, however added authentication here.

Previous dcr endpoints were unsecure, thought there are uses of that, however added authentication [here](https://repository.entgra.net/community/device-mgt-core/src/commit/40a5b00f80200fa894f56246e922922f0830e4a1/components/apimgt-extensions/io.entgra.device.mgt.core.apimgt.application.extension/src/main/java/io/entgra/device/mgt/core/apimgt/application/extension/APIManagementProviderServiceImpl.java#L319).
<Bundle-Description>API Management Application Bundle</Bundle-Description>
<Private-Package>io.entgra.device.mgt.core.apimgt.application.extension.internal</Private-Package>
<Import-Packages>
com.google.gson.*;version="${google.gson.version}",
Owner

Do not use wildcard imports here. Use exact package name.

Do not use wildcard imports here. Use exact package name.
return username;
}
public void setUsername(String username) {
Owner

Add method comment specifying this method require FQUN and will extracts the tenant domain from FQUN. According to logic, if user name provided without a tenant domain, it will assign to super tenant. So developer has to be aware.

Add method comment specifying this method require FQUN and will extracts the tenant domain from FQUN. According to logic, if user name provided without a tenant domain, it will assign to super tenant. So developer has to be aware.
private static final OkHttpClient client = new OkHttpClient(HttpsTrustManagerUtils.getSSLClient().newBuilder());
private static final MediaType JSON = MediaType.parse("application/json; charset=utf-8");
private static final Gson gson = new Gson();
private static final String host = System.getProperty(Constants.IOT_CORE_HOST);
Owner

Shouldn't this call APIG MGT host? In a clustered environment with a seperate APIG profile, the request will be sent to IoT Core according to this.

Shouldn't this call APIG MGT host? In a clustered environment with a seperate APIG profile, the request will be sent to IoT Core according to this.
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
public class OAuthClient implements IOAuthClientService {
Owner

Use synchronized blocks and appropriate singletons to safeguard critical paths of the flow. Make sure there will be no parallel executions of critical paths such as api application creations, update, token generation, renewal etc.

Use synchronized blocks and appropriate singletons to safeguard critical paths of the flow. Make sure there will be no parallel executions of critical paths such as api application creations, update, token generation, renewal etc.
rajitha added 1 commit 2 weeks ago

Reviewers

charithag requested changes 2 weeks ago
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: community/device-mgt-core#536
Loading…
There is no content yet.