Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compatability for jpackaged modular applications #571

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions msal4j-sdk/bnd.bnd
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
Export-Package: com.microsoft.aad.com.microsoft.aad.msal4j
Automatic-Module-Name: com.microsoft.aad.msal4j

-fixupmessages: \ "Classes found in the wrong directory...";is:=warning
51 changes: 42 additions & 9 deletions msal4j-sdk/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
<dependency>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
<version>1.18.6</version>
<version>1.18.24</version>
<scope>provided</scope>
</dependency>
<dependency>
Expand Down Expand Up @@ -161,7 +161,7 @@
<plugin>
<groupId>org.projectlombok</groupId>
<artifactId>lombok-maven-plugin</artifactId>
<version>1.18.2.0</version>
<version>1.18.20.0</version>
<executions>
<execution>
<goals>
Expand All @@ -179,9 +179,12 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-jar-plugin</artifactId>
<version>2.5</version>
<version>3.2.0</version>
<configuration>
<archive>
<manifestEntries>
<Multi-Release>true</Multi-Release>
</manifestEntries>
<manifest>
<addDefaultImplementationEntries>true</addDefaultImplementationEntries>
<addDefaultSpecificationEntries>true</addDefaultSpecificationEntries>
Expand Down Expand Up @@ -236,11 +239,41 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<version>3.7.0</version>
<configuration>
<source>8</source>
<target>8</target>
</configuration>
<version>3.10.1</version>
<executions>
<execution>
<id>default-testCompile</id>
<goals>
<goal>testCompile</goal>
</goals>
<configuration>
<release>9</release>
</configuration>
</execution> <execution>
<id>default-compile</id>
<goals>
<goal>compile</goal>
</goals>
<configuration>
<source>8</source>
<target>8</target>
</configuration>
</execution>
<execution>
<id>compile-java-9</id>
<phase>compile</phase>
<goals>
<goal>compile</goal>
</goals>
<configuration>
<release>9</release>
<compileSourceRoots>
<compileSourceRoot>${project.basedir}/src/main/java9</compileSourceRoot>
</compileSourceRoots>
<multiReleaseOutput>true</multiReleaseOutput>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
Expand Down Expand Up @@ -277,7 +310,7 @@
<plugin>
<groupId>biz.aQute.bnd</groupId>
<artifactId>bnd-maven-plugin</artifactId>
<version>4.3.1</version>
<version>6.3.1</version>
<executions>
<execution>
<goals>
Expand Down
126 changes: 126 additions & 0 deletions msal4j-sdk/src/main/java9/com/microsoft/aad/msal4j/HttpHeaders.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@siddhijain @Avery-Dunn - please try to respond to community contributions. This looks ok to me, although it's missing tests. What is the context here? I don't see any feedback on this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, Avery and I talked about this PR, but we never responded.

Context - We use a method getProductVersion() to get the msal4j version, and it returns 1 by default (i.e., when it cannot find the implementation version, for eg when running tests or in a jpackaged modular application). According to the person who raised PR, it is throwing exceptions while using IWA flow.
The PR adds another version of the HttpHeaders class that uses Java 9 features to handle this error. However, we avoid maintaining different versions of a class for different Java versions due to reasons like increased code complexity, code duplication, incompatibility issues, upgrade difficulties, and many more. 

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it seems this PR won't be merged, is there any fix you'd propose?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RJPG-TK For the time being, please continue using the workaround you provided in your message above, as we find a solution to the issue.

// Licensed under the MIT License.

package com.microsoft.aad.msal4j;

import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.function.BiConsumer;
import java.util.Optional;
import java.lang.module.ModuleDescriptor;

final class HttpHeaders {

static final String PRODUCT_HEADER_NAME = "x-client-SKU";
static final String PRODUCT_HEADER_VALUE = "MSAL.Java";

static final String PRODUCT_VERSION_HEADER_NAME = "x-client-VER";
static final String PRODUCT_VERSION_HEADER_VALUE = getProductVersion();

static final String CPU_HEADER_NAME = "x-client-CPU";
static final String CPU_HEADER_VALUE = System.getProperty("os.arch");

static final String OS_HEADER_NAME = "x-client-OS";
static final String OS_HEADER_VALUE = System.getProperty("os.name");

static final String APPLICATION_NAME_HEADER_NAME = "x-app-name";
private final String applicationNameHeaderValue;

static final String APPLICATION_VERSION_HEADER_NAME = "x-app-ver";
private final String applicationVersionHeaderValue;

static final String CORRELATION_ID_HEADER_NAME = "client-request-id";
private final String correlationIdHeaderValue;

private static final String REQUEST_CORRELATION_ID_IN_RESPONSE_HEADER_NAME = "return-client-request-id";
private static final String REQUEST_CORRELATION_ID_IN_RESPONSE_HEADER_VALUE = "true";

private static final String X_MS_LIB_CAPABILITY_NAME = "x-ms-lib-capability";
private static final String X_MS_LIB_CAPABILITY_VALUE = "retry-after, h429";

// Used for CCS routing
static final String X_ANCHOR_MAILBOX = "X-AnchorMailbox";
static final String X_ANCHOR_MAILBOX_OID_FORMAT = "oid:%s";
static final String X_ANCHOR_MAILBOX_UPN_FORMAT = "upn:%s";
private String anchorMailboxHeaderValue = null;

private String headerValues;
private Map<String, String> headerMap = new HashMap<>();

HttpHeaders(final RequestContext requestContext) {
correlationIdHeaderValue = requestContext.correlationId();
applicationNameHeaderValue = requestContext.applicationName();
applicationVersionHeaderValue = requestContext.applicationVersion();

if (requestContext.userIdentifier() != null) {
String upn = requestContext.userIdentifier().upn();
String oid = requestContext.userIdentifier().oid();
if (!StringHelper.isBlank(upn)) {
anchorMailboxHeaderValue = String.format(X_ANCHOR_MAILBOX_UPN_FORMAT, upn);
} else if (!StringHelper.isBlank(oid)) {
anchorMailboxHeaderValue = String.format(X_ANCHOR_MAILBOX_OID_FORMAT, oid);
}
}

Map<String, String> extraHttpHeaders = requestContext.apiParameters() == null ?
null :
requestContext.apiParameters().extraHttpHeaders();
this.initializeHeaders(extraHttpHeaders);
}

private void initializeHeaders(Map<String, String> extraHttpHeaders) {
StringBuilder sb = new StringBuilder();

BiConsumer<String, String> init = (String key, String val) -> {
headerMap.put(key, val);
sb.append(key).append("=").append(val).append(";");
};

init.accept(PRODUCT_HEADER_NAME, PRODUCT_HEADER_VALUE);
init.accept(PRODUCT_VERSION_HEADER_NAME, PRODUCT_VERSION_HEADER_VALUE);
init.accept(OS_HEADER_NAME, OS_HEADER_VALUE);
init.accept(CPU_HEADER_NAME, CPU_HEADER_VALUE);
init.accept(REQUEST_CORRELATION_ID_IN_RESPONSE_HEADER_NAME, REQUEST_CORRELATION_ID_IN_RESPONSE_HEADER_VALUE);
init.accept(CORRELATION_ID_HEADER_NAME, this.correlationIdHeaderValue);

if (!StringHelper.isBlank(this.applicationNameHeaderValue)) {
init.accept(APPLICATION_NAME_HEADER_NAME, this.applicationNameHeaderValue);
}
if (!StringHelper.isBlank(this.applicationVersionHeaderValue)) {
init.accept(APPLICATION_VERSION_HEADER_NAME, this.applicationVersionHeaderValue);
}
if (!StringHelper.isBlank(this.anchorMailboxHeaderValue)) {
init.accept(X_ANCHOR_MAILBOX, this.anchorMailboxHeaderValue);
}

init.accept(X_MS_LIB_CAPABILITY_NAME, X_MS_LIB_CAPABILITY_VALUE);

if (extraHttpHeaders != null) {
extraHttpHeaders.forEach(init);
}

this.headerValues = sb.toString();
}

Map<String, String> getReadonlyHeaderMap() {
return Collections.unmodifiableMap(this.headerMap);
}

String getHeaderCorrelationIdValue() {
return this.correlationIdHeaderValue;
}

@Override
public String toString() {
return this.headerValues;
}

private static String getProductVersion() {
return Optional
.ofNullable(HttpHeaders.class.getModule().getDescriptor())
.flatMap(ModuleDescriptor::version)
.map(ModuleDescriptor.Version::toString)
.orElse("1.0");
}
}