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.

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

[ELY-201] Add client side HTTP authentication mechanism support #528

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
= ELY-201 Add client side HTTP authentication mechanism support.
:author: Keshav Kumar
:email: [email protected]
:toc: left
:icons: font
:idprefix:
:idseparator: -

== Overview

Wildfly Elytron already provide client and server side SASL mechanism for server side HTTP authentication mechanism.This will include a client side framework for HTTP authentication mechanisms and implement a set of client side HTTP authentication mechanisms.
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@jamezp we did not discuss the Authenticator usage. But it seems that for our use case where we need to obtain not just a password from the elytron configuration but also the ssl context, credentials stored in the credential store, and bearer tokens it is not so clear how the Authenticator would fit in. Let us know if you have any idea or a suggestion for it

Copy link
Member

Choose a reason for hiding this comment

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


== Issue Metadata

=== Issue

* https://issues.redhat.com/browse/ELY-201

=== Related Issues

* https://issues.redhat.com/browse/EAP7-666
* https://issues.redhat.com/browse/WFLY-17938

=== Dev Contacts

* mailto:{email}[{author}]

=== QE Contacts

=== Testing By
// Put an x in the relevant field to indicate if testing will be done by Engineering or QE.
// Discuss with QE during the Kickoff state to decide this
* [ ] Engineering
Copy link
Contributor

Choose a reason for hiding this comment

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

This will be tested by engineering AFAIK


* [ ] QE

=== Affected Projects or Components
Copy link
Contributor

Choose a reason for hiding this comment

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

affected projects or components are wildfly-elytron and wildfly


=== Other Interested Projects

=== Relevant Installation Types
// Remove the x next to the relevant field if the feature in question is not relevant
// to that kind of WildFly installation
* [x] Traditional standalone server (unzipped or provisioned by Galleon)

* [x] Managed domain

* [x] OpenShift s2i

* [x] Bootable jar

== Requirements

=== Hard Requirements

Having a client which can read the elytron configurations and add authorization header with BASIC,BEARER or DIGEST based on challenge it received.
Copy link
Contributor

Choose a reason for hiding this comment

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

please add using an SSLContext from the elytron configuration tot his list


=== Nice-to-Have Requirements

Copy link
Contributor

Choose a reason for hiding this comment

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

You can put http-selector in the nice to have requirements section

=== Non-Requirements

== Backwards Compatibility

Copy link
Contributor

Choose a reason for hiding this comment

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

You can write here that it does not affect backwards compatibility because we did not provide any HTTP client before

// Does this enhancement affect backwards compatibility with previously released
// versions of WildFly?
// Can the identified incompatibility be avoided?

=== Default Configuration

=== Importing Existing Configuration

=== Deployments

=== Interoperability

//== Implementation Plan

HttpClient provides a connect() method which is used to send a request and a response will be received.
connect() method require parameters :-
keshav-redhat marked this conversation as resolved.
Show resolved Hide resolved

I) URI – URI which user wants to connect.
keshav-redhat marked this conversation as resolved.
Show resolved Hide resolved
2) RequestMethod
3) Body – request body to be attached with uri in case of post,put request method.
4) Headers
Working Module : http/client
keshav-redhat marked this conversation as resolved.
Show resolved Hide resolved

We will create a class called ElytronHttpClient which contains method connect().

We will create an object of HttpClient in the constructor of ElytronHttpClient.

[source,xml]
----
public ElytronHttpClient(){
   client = HttpClient.newHttpClient();
keshav-redhat marked this conversation as resolved.
Show resolved Hide resolved
}
----

connect() method requires a parameter uri which is of type String,In this method we first get an instance of HttpRequest using buildRequest() method and using HttpClient object we will send the request by send() method which return HttpResponse.

public HttpResponse<String> connect(String uri,String method, String body, Map<String, String> headers)
keshav-redhat marked this conversation as resolved.
Show resolved Hide resolved

keshav-redhat marked this conversation as resolved.
Show resolved Hide resolved

-> In case of Basic Authentication mechanism,we require username and password to authenticate the user for the given uri.For that we will add user credential to the wildfly-config.xml as following :-
keshav-redhat marked this conversation as resolved.
Show resolved Hide resolved

[source,xml]
----
<set-user-name name="quickstartUser"/>
<credentials>
   <clear-password password="quickstartPwd1!"/>
</credentials>
----

We will use different type of authentication mechanism 
Basic,bearer,certificate,digest,external
keshav-redhat marked this conversation as resolved.
Show resolved Hide resolved

Based on the mechanism type we will call different methods to get authorization value which will be added to the header for authentication.
Like for basic type authentication mechanism we will create a class which contains evaluateMechanism() method :  

[source,xml]
----
public static HttpRequest evaluateMechanism(URI uri) {
HttpRequest request = HttpRequest
.newBuilder()
.uri(uri)
.header(ElytronHttpClientConstants.AUTHORIZATION, basicAuth(userName, password))
.build();
return request;
}
----

Similar to this we will have different classes for different authentication mechanism like digest,bearer,etc.

Now after sending the request we will get a response which is of type HttpResponse.we will determine using the status if the authentication success or failed.If status code is 200 then we will know that authentication is successful.

If the authentication failure then headers will contain : 
www-authenticate=[Basic realm="RealmUsersRoles"] for basic authentication.similar for the other authentication mechanism type like digest,bearer,etc.
Copy link
Contributor

Choose a reason for hiding this comment

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

It is not clear what happens after we receive authentication failure. If the authentication results in failure then we create another request created from the needed information and send this new request to the same URI.

Also what is failure in this context? If we receive response code 500 then it does not make sense to try to add credentials? Can you write here whether by authentication failure do you mean codes 401 or 403 or other?



== Security Considerations

////
Identification if any security implications that may need to be considered with this feature
or a confirmation that there are no security implications to consider.
////

== Test Plan

To test the above functionality we can add the tests as follows : 
Inside tests/base/src/test/java/org/wildfly/security/http/client/ we will create a test class named as ElytronHttpClientTest where we will get an instance of HttpRequest from ElytronHttpClient#evaluateMechanism() method of different authentication mechanism and then we will create an object of TestingHttpServerRequest using request header which we will pass to the evaluateRequest() method of BasicAuthenticationMechanism which is used to evaluate our header values of request method.
Copy link
Contributor

Choose a reason for hiding this comment

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

I am having difficulties understanding this long sentence. Can you please reformulate this? Also by object of TestingHttpServerRequest you mean instance of right? since the class already exists. Also it is not clear when you are refering to the authentication mechanism of the client and when you are refering to the authentication mechanism of the server


keshav-redhat marked this conversation as resolved.
Show resolved Hide resolved
[source,xml]
----
TestingHttpServerRequest testingHttpServerRequest = new TestingHttpServerRequest(new String[]{request.headers().allValues("Authorization").get(0)});
----

The actual communication flow and connect method have be tested with arquillian in wildfly where we will call the connect method with required parameter and test it.

== Community Documentation
////
Copy link
Contributor

Choose a reason for hiding this comment

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

Community documentation has to be added to the wildfly repository under docs older

Copy link
Author

Choose a reason for hiding this comment

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

@Skyllarr ,I did not get this comment.

Copy link
Contributor

@Skyllarr Skyllarr Jul 5, 2023

Choose a reason for hiding this comment

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

@keshav-redhat new features are being documented in the upstream wildfly documentation that is located in the docs folder

Generally a feature should have documentation as part of the PR to wildfly master, or as a follow up PR if the feature is in wildfly-core. In some cases though the documentation belongs more in a component, or does not need any documentation. Indicate which of these will happen.
////
== Release Note Content
////
Draft verbiage for up to a few sentences on the feature for inclusion in the
Copy link
Contributor

Choose a reason for hiding this comment

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

Try to draft here a release note content. That should include a brief description of this feature

Release Note blog article for the release that first includes this feature.
Example article: http://wildfly.org/news/2018/08/30/WildFly14-Final-Released/.
This content will be edited, so there is no need to make it perfect or discuss
what release it appears in. "See Overview" is acceptable if the overview is
suitable. For simple features best covered as an item in a bullet-point list
of features containing a few words on each, use "Bullet point: <The few words>"
////