sk-eid / mid-rest-java-client Goto Github PK

The MidHashToSign does not declare a serialVersionUID so session deserialization might break in the future.

Please add a serialVersionUID to https://github.com/SK-EID/mid-rest-java-client/blob/master/src/main/java/ee/sk/mid/MidHashToSign.java

I would create a PR myself but I don't have the access rights

MidAuthenticationResponseValidator loads trusted CA certificates from a classpath resource called trusted_certificates.jks located in resources directory of the library. However, when SmartID Java Client is also present on the classpath, another truststore with the same name is defined on the classpath and it is undetermined if MID Java Client loads the correct truststore or not.

My suggestion would be to rename trusted_certificates.jks to a more specific mid_trusted_certificates.jks.

Additionally, there is no method like clearTrustedCACertificates() in SmartID Java Client to remove the certificates loaded by default in case I want to rely purely on the ones added via addTrustedCACertificate(..).

The expired certificate can be found in MidClient.java. This is nothing critical as there are several ways to specify a newer certificate, but still causes a bit of set back.

I couldn't help but notice that as of version 1.5 this project was migrated to Jersey 3.0.x, which uses the jakarta.ws namespace, and at the same time it and depends on javax.validation for bean validation.

Or sort of depends. The classes are annotated with @javax.validation.NotNull yet I found no place with @javax.validation.Valid, no dependency on Jersey Bean Validation extension nor an explicit call to a javax.validation.Validator anywhere in production code.

Regardless, is this a combination worth supporting? I imagine migrating from Java EE to Jakarta EE is something that ought to be done at once to highlight places needing attention. Or was the @javax.validation.NotNull annotation used for documentation/static analysis purposes and it may be replaced with something else (e.g. JetBrains annotations that are already used here as well)?

Should this be logged as error level?
https://github.com/SK-EID/mid-rest-java-client/blob/master/src/main/java/ee/sk/mid/MidAuthenticationResponseValidator.java#L162
Would recommend logging as debug level if needed.

With current implementation proxy settings will only work with Jersy JAXRS provider.
Provide a way to pass configuration to SmartIdClient that will work with all implementations of JAXRS.
Issue is that Proxy is not specified by JAXRS, instead maybe it would be better to provide a way to pass Client to SmartIdClient.
With current implementation any ClientConfig settings can not be used on WildFly or Jboss.

Do you want to request a feature or report a bug?
Report a bug.

What is the current behavior?
When internet connection drops during MidConnector.getSessionStatus, no response or exception get returned and code execution hangs.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem.
Step 1: Start authentication process.
Step 2: When your phone gets the notification, drop your internet connection.
If you attach debugger on line 71 in MidSessionStatusPoller.class, you will notice that java is waiting for a response from this.connector.getSessionStatus.

package ee.sk.mydata;

import ee.sk.mid.*;
import ee.sk.mid.rest.dao.MidSessionStatus;
import ee.sk.mid.rest.dao.request.MidAuthenticationRequest;
import ee.sk.mid.rest.dao.response.MidAuthenticationResponse;

public class MidAuthenticator {
    private MidClient midClient;

    public MidAuthenticator() {
        midClient = MidClient.newBuilder()
                .withRelyingPartyUUID("00000000-0000-0000-0000-000000000000")
                .withRelyingPartyName("DEMO")
                .withHostUrl("https://tsp.demo.sk.ee/mid-api")
                .withLongPollingTimeoutSeconds(5)
                .build();
    }

    public MidAuthentication authenticateUser(MidAuthenticationHashToSign authenticationHash,
                                              String identityNumber, String phoneNumber) {
        MidAuthenticationRequest request = MidAuthenticationRequest.newBuilder()
                .withPhoneNumber(phoneNumber)
                .withNationalIdentityNumber(identityNumber)
                .withHashToSign(authenticationHash)
                .withLanguage(MidLanguage.ENG)
                .withDisplayText("Log into self-service?")
                .withDisplayTextFormat(MidDisplayTextFormat.GSM7)
                .build();

        MidAuthenticationResponse response = midClient.getMobileIdConnector().authenticate(request);

        return midClient.createMobileIdAuthentication(getSessionStatus(response), authenticationHash);

    }

    MidSessionStatus getSessionStatus(MidAuthenticationResponse response) {
        return midClient.getSessionStatusPoller()
                .fetchFinalSessionStatus(response.getSessionID(), "/authentication/session/{sessionId}");

    }
}

public static void main(String[] args) {
        MidAuthenticator authenticator = new MidAuthenticator();
        MidAuthenticationHashToSign authenticationHash = MidAuthenticationHashToSign.generateRandomHashOfDefaultType();
        String identityNumber = null; // Replace value
        String phoneNumber = null; // Replace value
        try {
            authenticator.authenticateUser(authenticationHash, identityNumber, phoneNumber);
        } catch (Exception e) {
            System.err.println("Code executed correctly");
        }
    }

What is the expected behavior?
MidSessionStatusPoller.pollForFinalSessionStatus should not wait longer than
longPollingTimeoutSeconds value to ask for the sessions status.