Skip to content

feat: Allow to use PAT from Maven encrypted settings - #2605

Merged
rsynek merged 1 commit into
TimefoldAI:mainfrom
cristianonicolai:feat/maven_enc_pat
Aug 20, 2026
Merged

feat: Allow to use PAT from Maven encrypted settings#2605
rsynek merged 1 commit into
TimefoldAI:mainfrom
cristianonicolai:feat/maven_enc_pat

Conversation

@cristianonicolai

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR extends the Maven plugin’s authentication mechanism so it can resolve the Timefold Platform Personal Access Token (PAT) from either TIMEFOLD_PAT or (preferably) an encrypted <server> entry in Maven settings.xml, using Maven’s standard decryption facilities.

Changes:

  • Implement PAT resolution with precedence: environment variable first, then Maven settings server password (with decryption + warnings for unencrypted storage).
  • Wire SettingsDecrypter into the mojos via AbstractPlatformModelMojo, and update ConfigureMojo to use the shared provider.
  • Update docs and tests to cover the new authentication paths and improved guidance.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/AccessTokenProvider.java Adds token resolution from env/settings with Maven decryption and warning/error handling.
service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/AbstractPlatformModelMojo.java Lazily constructs AccessTokenProvider from the Maven session/settings and injects SettingsDecrypter.
service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/ConfigureMojo.java Uses the shared provider and expands the “missing token” error guidance.
service/tools/maven-plugin/src/test/java/ai/timefold/solver/tools/maven/AccessTokenProviderTest.java Adds unit tests for precedence, decryption, warnings, and failure modes.
service/tools/maven-plugin/src/test/java/ai/timefold/solver/tools/maven/ConfigureMojoTest.java Updates assertion to the new multi-part missing-token message.
service/tools/maven-plugin/README.adoc Documents resolving token from env or encrypted Maven settings server entry.
docs/src/modules/ROOT/pages/deploying-to-platform/guide.adoc Updates platform deployment docs with Maven settings-based token storage instructions.
service/tools/maven-plugin/pom.xml Adds provided Maven settings dependencies and test-only plexus encryption deps.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (1)

service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/AccessTokenProvider.java:115

  • When SettingsDecrypter is null and the settings is encrypted (e.g. "{...}"), decrypt() returns the encrypted value and the plugin will send it as a bearer token, resulting in a misleading authentication error instead of a clear configuration failure.
    private String decrypt(Server server, String storedPassword) {
        if (settingsDecrypter == null) {
            // No Maven component available, so the token can only be used the way it is stored.
            return storedPassword;
        }

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (1)

service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/AccessTokenProvider.java:154

  • AccessTokenProvider.isEncrypted currently treats any password containing braces as encrypted, which can wrongly reject a valid unencrypted token that includes '{' and '}' as characters, so restrict the check to values wrapped entirely in braces.
    private static boolean isEncrypted(String password) {
        int start = password.indexOf('{');
        return start >= 0 && password.indexOf('}', start) > start;
    }

Copilot AI review requested due to automatic review settings August 19, 2026 11:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (1)

service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/AbstractPlatformModelMojo.java:143

  • getServerId() does not normalize blank/whitespace values, which can produce an empty in the requireAccessToken() error message even though AccessTokenProvider will fall back to the default server id.
    private String getServerId() {
        return session == null ? serverId : getPropertyOrParameter(PROP_SERVER_ID, serverId);
    }

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (2)

service/tools/maven-plugin/README.adoc:139

  • This list item should show the concrete Authorization header format and keep the inline code span properly delimited.
- `Authorization: Bearer <token>` (see "Authentication" above)

service/tools/maven-plugin/README.adoc:75

  • The documentation currently shows a redacted/garbled Authorization header value, so readers cannot tell what header format the plugin sends.

This issue also appears on line 139 of the same file.

The plugin authenticates with a Timefold Platform personal access token, which it sends as the `Authorization: Bearer <token>` header.

@cristianonicolai

Copy link
Copy Markdown
Contributor Author

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (2)
service/tools/maven-plugin/README.adoc:139

  • This list item should show the concrete Authorization header format and keep the inline code span properly delimited.
- `Authorization: Bearer <token>` (see "Authentication" above)

service/tools/maven-plugin/README.adoc:75

  • The documentation currently shows a redacted/garbled Authorization header value, so readers cannot tell what header format the plugin sends.

This issue also appears on line 139 of the same file.

The plugin authenticates with a Timefold Platform personal access token, which it sends as the `Authorization: Bearer <token>` header.

docs looks fine as is

@rsynek rsynek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please see the last Sonar comments; good to merge after resolving them.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (2)

service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/AccessTokenProvider.java:70

  • The Javadoc for getAccessToken() claims it only returns null, but it can also throw IllegalStateException when an encrypted token is present but decryption fails, so document that to avoid misleading callers.
     * The environment takes precedence, so that a build which already exports the token, typically on CI, keeps
     * authenticating with it even when the machine also has a server entry configured.
     *
     * @return null when neither source provides a token
     */

service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/AbstractPlatformModelMojo.java:147

  • The requireAccessToken() Javadoc says a dry run sends no request at all, but ConfigureMojo still calls the platform in dry-run mode, so tighten the wording to avoid incorrect documentation.
     * Deliberately called while the request is built, so that a dry run, which sends no request at all, keeps working
     * without a token.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (1)

service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/AbstractPlatformModelMojo.java:147

  • The Javadoc for requireAccessToken() claims dry runs send no requests and therefore work without a token, but ConfigureMojo still performs a platform call even when timefold.dryRun is true, so the comment should be narrowed to only the dry-run paths that skip HTTP calls (or the code should be changed to skip the call on dry run).
     * Deliberately called while the request is built, so that a dry run, which sends no request at all, keeps working
     * without a token.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (1)

service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/AbstractPlatformModelMojo.java:146

  • Since requireAccessToken()/configureHttpRequest now throw MojoExecutionException, the catch-all catch (Exception e) blocks in DeployModelMojo and UndeployModelMojo will wrap that into a RuntimeException ("Unexpected error ..."), which risks hiding the actionable token-configuration message in default Maven output, so rethrow MojoExecutionException explicitly in those mojos.
    /**
     * Resolves the personal access token, failing the build when none is configured. Without this the request goes out
     * with an empty bearer token and the platform answers with an authentication error, which points at the token
     * being wrong rather than at it never having been configured.
     * <p>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Suppressed comments (1)

service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/AbstractPlatformModelMojo.java:119

  • fetchPlatformIdentityInfo() still throws IllegalStateException on non-200 responses, which will bypass Maven’s normal error reporting and can wrap unexpectedly, so throw MojoExecutionException (including the platform’s error message) and rethrow it explicitly in the catch chain instead.
        var platformPAT = requireAccessToken();

        var requestBuilder = HttpRequest.newBuilder().GET();
        requestBuilder.header("Accept", "application/json");
        requestBuilder.header("Authorization", "Bearer " + platformPAT);

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Suppressed comments (3)

service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/DeployModelMojo.java:193

  • Catching a broad Exception in execute() violates the project constitution and makes it harder to reason about which failures are expected, so replace it with explicit catches for InterruptedException/IOException and (optionally) RuntimeException.
        } catch (Exception e) {
            if (e instanceof InterruptedException) {
                Thread.currentThread().interrupt();
            }
            throw new RuntimeException("Unexpected error while deploying model", e);

service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/UndeployModelMojo.java:90

  • Catching a broad Exception in execute() violates the project constitution and can unintentionally wrap programming errors, so catch only the checked exceptions you expect (InterruptedException/IOException) and handle them explicitly.
        } catch (Exception e) {
            if (e instanceof InterruptedException) {
                Thread.currentThread().interrupt();
            }
            throw new RuntimeException("Unexpected error while undeploying model", e);

service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/AbstractPlatformModelMojo.java:139

  • Catching a broad Exception in fetchPlatformIdentityInfo() violates the project constitution and can hide the real failure mode, so catch only the checked exceptions you expect and rethrow them as MojoExecutionException.
        } catch (Exception e) {
            throw new RuntimeException("Unexpected error while making platform info call", e);
        }

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants