Skip to content

Enable deploy from url with token - #1928

Open
s-yonkov-yonkov wants to merge 4 commits into
masterfrom
feature/LMCROSSITXSADEPLOY-3391
Open

s-yonkov-yonkov wants to merge 4 commits into
masterfrom
feature/LMCROSSITXSADEPLOY-3391

Conversation

@s-yonkov-yonkov

Copy link
Copy Markdown
Contributor

No description provided.

…ROSSITXSADEPLOY-3391]

Add bearer token support to the deploy-from-URL feature so MTA archives
hosted on GitHub (release assets, raw content) can be fetched using a
GitHub PAT or GITHUB_TOKEN.

Changes:
- UserCredentials: add optional `token` field (additive, backward compatible)
- Introduce RequestAuthenticator strategy (Bearer, Basic, UrlUserInfo, None)
  with explicit precedence: token > basic > url-userinfo > none
- AuthenticatorResolver injected as a @nAmed Spring bean into
  DeployFromUrlRemoteClient (replaces inline if/else ladder)
- Fix pre-existing audit-log leak: logStartUploadFromUrl now receives the
  userinfo-stripped URL instead of the raw decoded URL
- Tests: AuthenticatorResolverTest (9), DeployFromUrlRemoteClientTest (+2),
  FilesApiServiceImplTest (+1 audit-log regression test)

# Conflicts:
#	multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/client/DeployFromUrlRemoteClient.java
#	multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/client/DeployFromUrlRemoteClientTest.java
@s-yonkov-yonkov
s-yonkov-yonkov force-pushed the feature/LMCROSSITXSADEPLOY-3391 branch from a26345c to e0c63ee Compare October 16, 2026 13:36
@sonarqubecloud

Copy link
Copy Markdown

Comment on lines +26 to +28
public static final String DEPLOY_FROM_URL_FORBIDDEN_FOR_JOB_WITH_ID = "Access to {0} is forbidden. Make sure the provided token has the required permissions.";
public static final String DEPLOY_FROM_URL_NOT_FOUND_WITH_TOKEN_FOR_JOB_WITH_ID = "The resource at {0} was not found. If it is private, verify the provided token is valid and has read access.";
public static final String DEPLOY_FROM_URL_CONNECTION_FAILED = "Failed to connect to the remote endpoint. Check the URL is reachable and the server is available.";

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.

The varaible names don't match the value. Can you rename them


public RequestAuthenticator resolve(UserCredentials userCredentials, String decodedUrl) {
if (userCredentials != null && userCredentials.getToken() != null) {
return new BearerTokenAuthenticator(userCredentials.getToken());

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.

Is there a case in which this token can be empty string?

}

private boolean hasToken(UserCredentials userCredentials) {
return userCredentials != null && userCredentials.getToken() != null;

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.

Can the token be empty string?

Comment on lines +25 to +30
if (userCredentials != null && userCredentials.getToken() != null) {
return new BearerTokenAuthenticator(userCredentials.getToken());
}
if (userCredentials != null) {
return new BasicAuthenticator(userCredentials.getUsername(), userCredentials.getPassword());
}

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.

the (userCredentials != null) check is repeated. Can you remove the duplication by combining the if-s

if (e instanceof SLException) {
return e.getMessage();
}
return Messages.DEPLOY_FROM_URL_CONNECTION_FAILED;

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.

Won't we hide important information as why the deployment failed if we mask the exception message?

This branch has not been deployed

No deployments
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.

2 participants