Skip to content

Integration test for custom authenticator in internal user login success flow #23661

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

indeewari
Copy link
Contributor

@indeewari indeewari commented Apr 1, 2025

This PR implements the success path for setting up and authenticating a consumer user using a custom authenticator for Internal User Identification Authenticators.

  • An admin configures a custom authenticator for internal users, with a mock service serving as the authenticator and storing internal user data.
  • A consumer user is created as an internal user, and their information is shared with the mock service as a mock step.
  • The consumer user authenticates through an application integrated with the custom authenticator, and the received ID token is validated against expected claims.

Parent issue:

@indeewari indeewari changed the title Integration test for custom authenticator Integration test for custom authenticator in internal user login success flow Apr 1, 2025
@@ -0,0 +1,583 @@
package org.wso2.identity.integration.test.rest.api.server.authenticator.execution.v1;
Copy link
Member

Choose a reason for hiding this comment

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

This is an integration test for the authenticator execution flow right ? why we have this under rest.api.server package ?

Comment on lines +328 to +334
/**
* Extracts the JWT claims set from a given JWT token.
*
* @param jwtToken JWT token from which claims are to be extracted
* @return JWTClaimsSet extracted from the provided JWT token
* @throws ParseException If there is an error in parsing the JWT token
*/
Copy link
Member

Choose a reason for hiding this comment

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

IMO we don't need these comments. Method is self explanatory

* @param consumerSecret Consumer secret of the application.
* @return Base 64 encoded string.
*/
public String getBase64EncodedString(String consumerKey, String consumerSecret) {
Copy link
Member

Choose a reason for hiding this comment

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

why is this public ? Also check if there's any util

return customAuthenticatorAuthRQ;
}

public HttpResponse sendGetRequest(HttpClient client, String locationURL) throws IOException {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think any of these methods should be public

AUTHENTICATOR_CONFIG_API_BASE_PATH + customIdPId, tenant)));
}

protected UserDefinedLocalAuthenticatorConfig createBaseUserDefinedLocalAuthenticator(
Copy link
Member

Choose a reason for hiding this comment

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

Are we expecting this test to be extended ?

private String identityProviderBaseUrl;
private UserDTO internalUser;

public void start(String identityProviderBaseUrl, UserDTO internalUser) {
Copy link
Member

Choose a reason for hiding this comment

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

Should we say internalUser ?
I think the same service can be used to model other cases as well

Comment on lines +50 to +51
System.out.println("Received Request: " + request);
System.out.println("Response Sent: " + response.getBodyAsString());
Copy link
Member

Choose a reason for hiding this comment

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

Can't we use some logger here than sys out ?

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