Fix the use of multiple credential providers #8909

evandongen · 2025-05-01T11:55:14Z

evandongen · 2025-05-01T11:59:07Z

CONTRIBUTING.md

@@ -263,8 +263,9 @@ This guide assumes that you are using IntelliJ Ultimate, because application ser
      - Set the context to `/iaf-test`.
 		__NB__: This is very important, otherwise a lot of tests will fail!
      - Set the following VM options:
-        `-Ddtap.stage=LOC -DauthAliases.expansion.allowed=testalias -Dweb.port=8080 -DcredentialFactory.class=org.frankframework.credentialprovider.FileSystemCredentialFactory -DcredentialFactory.filesystem.root=/<path to source>/frankframework/test/src/main/secrets`
-      - In the "On Update" section, select "Update Classes and Resources", so classes can be automatically updated and reloaded after project build (providing this is supported by your JDK)
+        `-Ddtap.stage=LOC -DauthAliases.expansion.allowed=testalias -Dweb.port=8080 


De filesystem credentials zijn een half jaar geleden verwijderd, maar de docs waren niet bijgewerkt.

evandongen · 2025-05-01T12:00:01Z

core/src/main/java/org/frankframework/jdbc/datasource/ObjectCreator.java

-	/**
-	 * Performs a 'safe' lookup of credentials.
-	 */
-	private CredentialFactory getCredentials(FrankResource resource) {


Alias werd hier resolved, maar werd hierboven eigenlijk niet meer gebruikt

evandongen · 2025-05-01T12:00:26Z

core/src/main/java/org/frankframework/management/bus/endpoints/SecurityItems.java


 			String userName;
 			String passWord;
 			try {
+				CredentialFactory cf = new CredentialFactory(authAlias);


Door mijn wijziging kan dit al een nosuchelementexception throwen.

evandongen · 2025-05-01T12:01:03Z

core/src/main/java/org/frankframework/pipes/CredentialCheckingPipe.java

@@ -59,6 +59,9 @@ public PipeRunResult doPipe(Message message, PipeLineSession session) throws Pip
 			result += "username does not match target";
 		}
 		if (!getTargetPassword().equals(cf.getPassword())) {
+			if (!StringUtils.isEmpty(result)) {


zonder deze change kwam hier username does not match targetpassword does not match target uit als beide niet klopten

evandongen · 2025-05-01T12:01:17Z

...ovider/src/main/java/org/frankframework/credentialprovider/AbstractMapCredentialFactory.java

@@ -83,7 +84,7 @@ public boolean hasCredentials(String alias) {
 	}

 	@Override
-	public ICredentials getCredentials(String alias, Supplier<String> defaultUsernameSupplier, Supplier<String> defaultPasswordSupplier) {
+	public ICredentials getCredentials(String alias, Supplier<String> defaultUsernameSupplier, Supplier<String> defaultPasswordSupplier) throws NoSuchElementException {


Feedback van vorige pr

evandongen · 2025-05-01T12:02:09Z

idin/src/test/java/org/frankframework/extensions/idin/IdinSenderTest.java

+	 * @throws ConfigurationException
+	 */
+	@Test
+	void testCertificateSettingsCanBeSet() throws ConfigurationException {


Bewijst dat alles netjes geset en configured kan wordne

idin/src/main/java/org/frankframework/extensions/idin/IdinSender.java

credentialProvider/src/main/java/org/frankframework/credentialprovider/CredentialFactory.java

nielsm5 · 2025-05-02T11:56:37Z

core/src/main/java/org/frankframework/jdbc/datasource/ObjectCreator.java

-		String alias = resource.getAuthalias();
-		if(StringUtils.isNotEmpty(alias)) {
-			alias = StringResolver.substVars(alias, APP_CONSTANTS);
-		}
-		String username = resource.getUsername();
-		if(StringUtils.isNotEmpty(username)) {
-			username = StringResolver.substVars(username, APP_CONSTANTS);
+		if(StringUtils.isNotEmpty(resource.getUsername())) {
+			mergedProps.setProperty("user", StringResolver.substVars(resource.getUsername(), APP_CONSTANTS));
 		}
-		String password = resource.getPassword();
-		if(StringUtils.isNotEmpty(password)) {
-			password = StringResolver.substVars(password, APP_CONSTANTS);
+		if(StringUtils.isNotEmpty(resource.getPassword())) {
+			mergedProps.setProperty("password", StringResolver.substVars(resource.getPassword(), APP_CONSTANTS));
 		}
-
-		return new CredentialFactory(alias, username, password);


Alias wordt toch meegegeven aan de CredentialFactory ?

nielsm5 · 2025-05-02T12:00:51Z

idin/src/main/java/org/frankframework/extensions/idin/IdinSender.java

+	private static class CredentialSettings {
+		private CredentialFactory credentialFactory;
+		private @Setter String password;
+		private @Getter String authAlias;
+
+		public void setAuthAlias(String authAlias) {
+			this.authAlias = authAlias;
+			this.credentialFactory = new CredentialFactory(authAlias);
+		}
+
+		public String getPassword() {
+			if (credentialFactory == null) {
+				return password;
+			}
+
+			return credentialFactory.getPassword();
+		}
+	}


Waarom gebruik je niet 3 class fields?

private @Getter String authAlias; private @Getter String username; private @Getter String password;

En in de configure v/d class credentialFactory = new CredentialFactory(authAlias, username, password); ?

Wat bedoel je met configure class?

Dan heb ik 9 class fields nodig, op deze manier (dat was ook feedback van Tim), houden we het nog een beetje netjes. Gebruik van een credential factory gaat mis zonder auth alias, vandaar deze wijziging

Dan kan je toch beter dit, geniek hier oplossen:

frankframework/core/src/main/java/org/frankframework/util/CredentialFactory.java

Lines 47 to 49 in b564600

public CredentialFactory(String alias, Supplier<String> defaultUsernameSupplier, Supplier<String> defaultPasswordSupplier) {

credentials = org.frankframework.credentialprovider.CredentialFactory.getCredentials(alias, defaultUsernameSupplier, defaultPasswordSupplier);

}

Zet daar om alias een mooie if, die een https://github.com/frankframework/frankframework/blob/master/credentialProvider/src/main/java/org/frankframework/credentialprovider/Credentials.java teruggeeft zonder alias?

(cherry picked from commit 30c8674)

…8904) (cherry picked from commit 17e7659)

…viders

core/src/main/java/org/frankframework/jdbc/datasource/ObjectCreator.java

…tps://github.com/frankframework/frankframework into issue/8879-support-multiple-credential-providers

sonarqubecloud · 2025-05-07T12:53:59Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
93.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

nielsm5 · 2025-05-07T13:08:25Z

core/src/main/java/org/frankframework/util/CredentialFactory.java

@@ -45,10 +46,13 @@ public CredentialFactory(String alias, String defaultUsername, String defaultPas
 	}

 	public CredentialFactory(String alias, Supplier<String> defaultUsernameSupplier, Supplier<String> defaultPasswordSupplier) {
-		credentials = org.frankframework.credentialprovider.CredentialFactory.getCredentials(alias, defaultUsernameSupplier, defaultPasswordSupplier);
+		if (alias == null) {
+			credentials = new Credentials(alias, defaultUsernameSupplier, defaultPasswordSupplier);


Ja dit is nu wat voorheen op CredentialFactory (in cf module) op regel 130 stond!

(cherry picked from commit 4b4fc26) # Conflicts: # core/src/main/java/org/frankframework/pipes/CredentialCheckingPipe.java # credentialProvider/src/main/java/org/frankframework/credentialprovider/delinea/DelineaCredentialFactory.java

(cherry picked from commit 4b4fc26)

closes #8879

39c7cf8

evandongen changed the title ~~closes #8879~~ Fix multiple credential providers May 1, 2025

evandongen commented May 1, 2025

View reviewed changes

evandongen requested review from nielsm5 and tnleeuw May 1, 2025 12:02

Fix unit test

fe49ff9

tnleeuw reviewed May 1, 2025

View reviewed changes

idin/src/main/java/org/frankframework/extensions/idin/IdinSender.java Show resolved Hide resolved

tnleeuw mentioned this pull request May 1, 2025

Test that verifies IdinSender auth-aliases work to resolve passwords for keystore / certificate stores. #8910

Closed

evandongen added 2 commits May 2, 2025 11:31

Integrate alias handling in IdinSender

70db15c

make a bit more generic (thanks Tim)

288ab82

evandongen requested a review from tnleeuw May 2, 2025 10:17

tnleeuw reviewed May 2, 2025

View reviewed changes

idin/src/main/java/org/frankframework/extensions/idin/IdinSender.java Outdated Show resolved Hide resolved

tnleeuw reviewed May 2, 2025

View reviewed changes

credentialProvider/src/main/java/org/frankframework/credentialprovider/CredentialFactory.java Show resolved Hide resolved

CredentialSettings may be static

643d0cb

nielsm5 requested changes May 2, 2025

View reviewed changes

evandongen and others added 4 commits May 2, 2025 16:04

Fix object creator

af7df81

Take advantage of the logcontext in pipe processors (#8902)

f284e74

(cherry picked from commit 30c8674)

Run LarvaTest scenarios individually in the Integration Test build (#…

058c6fb

…8904) (cherry picked from commit 17e7659)

Merge branch 'master' into issue/8879-support-multiple-credential-pro…

4abf6c7

…viders

evandongen requested review from nielsm5 and tnleeuw May 6, 2025 16:56

tnleeuw approved these changes May 7, 2025

View reviewed changes

nielsm5 reviewed May 7, 2025

View reviewed changes

core/src/main/java/org/frankframework/jdbc/datasource/ObjectCreator.java Outdated Show resolved Hide resolved

evandongen added 2 commits May 7, 2025 11:25

Merge branch 'issue/8879-support-multiple-credential-providers' of ht…

50307b7

…tps://github.com/frankframework/frankframework into issue/8879-support-multiple-credential-providers

Revert based on review comment

ee867bd

evandongen requested a review from a team as a code owner May 7, 2025 09:57

Revert ObjectCreator and change CredentialFactory

ca49520

nielsm5 reviewed May 7, 2025

View reviewed changes

nielsm5 approved these changes May 7, 2025

View reviewed changes

nielsm5 changed the title ~~Fix multiple credential providers~~ Fix the use of multiple credential providers May 7, 2025

nielsm5 merged commit 4b4fc26 into master May 7, 2025
32 of 33 checks passed

nielsm5 deleted the issue/8879-support-multiple-credential-providers branch May 7, 2025 13:09

evandongen added a commit that referenced this pull request May 15, 2025

Fix the use of multiple credential providers (#8909)

f478acc

(cherry picked from commit 4b4fc26)

	public CredentialFactory(String alias, Supplier<String> defaultUsernameSupplier, Supplier<String> defaultPasswordSupplier) {
	credentials = org.frankframework.credentialprovider.CredentialFactory.getCredentials(alias, defaultUsernameSupplier, defaultPasswordSupplier);
	}

Fix the use of multiple credential providers #8909

Fix the use of multiple credential providers #8909

Uh oh!

Conversation

evandongen commented May 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nielsm5 May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nielsm5 May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sonarqubecloud bot commented May 7, 2025

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

evandongen commented May 1, 2025 •

edited

Loading

nielsm5 May 2, 2025 •

edited

Loading

nielsm5 May 7, 2025 •

edited

Loading