Croway commented on PR #25876:
URL: https://github.com/apache/camel/pull/25876#issuecomment-5453811312
_AI-generated review comment by Codex on behalf of @Croway._
I found two issues with the authentication contract.
**P1 — the new ambiguity guard prevents the exact CAMEL-24495 auto-detection
case.** With `clientId`, `clientSecret`, `userName`, and no password,
`SalesforceLoginConfig` selects `CLIENT_CREDENTIALS`, but the Mojo rejects the
same inputs before creating the login configuration. This credential-free test
reproduces the mismatch:
```java
@Test
void mojoRejectsCredentialsThatSalesforceLoginConfigAutoDetects() {
SalesforceLoginConfig loginConfig = new SalesforceLoginConfig();
loginConfig.setClientId("client-id");
loginConfig.setClientSecret("client-secret");
loginConfig.setUserName("[email protected]");
assertThat(loginConfig.getType()).isEqualTo(AuthenticationType.CLIENT_CREDENTIALS);
AbstractSalesforceMojo mojo = new AbstractSalesforceMojo() {
@Override
protected AbstractSalesforceExecution getSalesforceExecution() {
throw new AssertionError("Validation should fail before an
execution is created");
}
};
mojo.clientId = "client-id";
mojo.clientSecret = "client-secret";
mojo.userName = "[email protected]";
mojo.password = null;
assertThatThrownBy(mojo::execute)
.isInstanceOf(MojoExecutionException.class)
.hasMessageContaining("Ambiguous authentication configuration");
}
```
I ran this against `2c23ecc27831`; it passes and confirms the divergence
without contacting Salesforce.
The minimal fix is to remove the `authenticationType == null && clientSecret
!= null && userName != null && password == null` rejection and let
`SalesforceLoginConfig.getType()` remain the single source of truth. `userName`
is not an authentication selector for Client Credentials; password presence
selects Username-Password, while its absence selects Client Credentials. A
small automated regression test should then assert that Mojo validation accepts
this combination (making `validateAuthenticationParameters()` package-private
would allow that without initiating a login).
**P2 — `REFRESH_TOKEN` is advertised but cannot work.** The generated Maven
parameter help lists `REFRESH_TOKEN` as supported, but the Mojo has no
`refreshToken` parameter and its validation only permits `clientSecret` or
`keystoreResource`. Please either implement that flow or remove it from the
supported-values description; the README currently correctly claims only the
three actually supported flows.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
