Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add api layer for api based authentication #2201

Merged
merged 1 commit into from
Oct 24, 2023
Merged

Add api layer for api based authentication #2201

merged 1 commit into from
Oct 24, 2023

Conversation

janakamarasena
Copy link
Contributor

@janakamarasena janakamarasena commented Oct 17, 2023 •
edited
Loading

API layer for wso2/product-is#15684

@janakamarasena janakamarasena force-pushed the api-auth-poc branch 3 times, most recently from a6fe3bc to 9ea1c2f Compare October 23, 2023 08:16
DMHP
DMHP reviewed Oct 23, 2023
View reviewed changes
...ndpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/api/auth/ApiAuthnEndpoint.java Outdated
params.put(AUTHENTICATOR, new String[]{authenticatorIdSplit[0]});
params.put(IDP, new String[]{authenticatorIdSplit[1]});
} else {
throw new AuthServiceClientException("Authenticator id is not in the correct format");
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's print the authenticator id.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

DMHP
DMHP reviewed Oct 23, 2023
View reviewed changes
...ndpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/api/auth/ApiAuthnEndpoint.java Outdated
LOG.debug("Client error while handling authentication request", e);
}
return buildOAuthInvalidRequestError(e.getMessage());
} catch (AuthServiceException | InvalidRequestParentException | URISyntaxException e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is InvalidRequestParentException a server error? The name of the class sounds different.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

its a client error. fixed.

DMHP
DMHP reviewed Oct 23, 2023
View reviewed changes
...ndpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/api/auth/ApiAuthnEndpoint.java Outdated

private final AuthenticationService authenticationService = new AuthenticationService();
private final OAuth2AuthzEndpoint oAuth2AuthzEndpoint = new OAuth2AuthzEndpoint();
private static final String AUTHENTICATOR_IDP_SPLITTER = ":";
Copy link
Contributor

Choose a reason for hiding this comment

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

Better move this constants to oAuthConstants

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

DMHP
DMHP reviewed Oct 23, 2023
View reviewed changes
...endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/api/auth/ApiAuthnHandler.java Outdated
List<Link> links = new ArrayList<>();
Link authnEpLink = new Link();
authnEpLink.setName("authentication");
String endpoint = "/oauth2/authn";
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's define these as constants in OAuthConstants.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed. Moved to class level constants as currently these are not used in any other place

DMHP
DMHP reviewed Oct 23, 2023
View reviewed changes
...ndpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/authz/OAuth2AuthzEndpoint.java Outdated
private boolean isApiBasedAuthenticationFlow(OAuth2Parameters oAuth2Parameters) {

if (oAuth2Parameters == null) {
log.debug("OAuth2Parameters is null. Returning false for isApiBasedAuthenticationFlow check.");
Copy link
Contributor

Choose a reason for hiding this comment

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

if (log.isDebugEnabled()) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@janakamarasena janakamarasena marked this pull request as ready for review October 23, 2023 10:32
@jenkins-is-staging
Copy link

PR builder started
Link: https://github.com/wso2/product-is/actions/runs/6613287968

@jenkins-is-staging
Copy link

PR builder completed
Link: https://github.com/wso2/product-is/actions/runs/6613287968
Status: failure

@jenkins-is-staging
Copy link

PR builder started
Link: https://github.com/wso2/product-is/actions/runs/6617584800

@jenkins-is-staging
Copy link

PR builder completed
Link: https://github.com/wso2/product-is/actions/runs/6617584800
Status: cancelled

@janakamarasena janakamarasena force-pushed the api-auth-poc branch 2 times, most recently from e041c61 to 595376e Compare October 23, 2023 19:16
DMHP
DMHP previously approved these changes Oct 24, 2023
View reviewed changes
@janakamarasena janakamarasena merged commit f3a8516 into wso2-extensions:master Oct 24, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DMHP DMHP DMHP approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@janakamarasena @jenkins-is-staging @DMHP