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

Fix #784 Enable BasicAuth through Selenide proxy server #785

Closed
wants to merge 3 commits into from
Closed
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.
+102 −1
Diff settings

Always

Just for now

@@ -57,7 +57,7 @@
<module name="UnnecessaryParentheses"/>
<!--<module name="MutableException"/>-->
<module name="ClassFanOutComplexity">
<property name="max" value="28"/>
<property name="max" value="31"/>

This comment has been minimized.

Copy link
@BorisOsipov

BorisOsipov Aug 21, 2018

Member

not related to this PR. We need to do smth with Selenide.java class

This comment has been minimized.

Copy link
@rosolko

rosolko Aug 21, 2018

Author Collaborator

Agree. It was one of the point to create separate AuthenticationType and Credentials classes to make logic easier a little bit.

</module>
<module name="CyclomaticComplexity">
<property name="max" value="20"/>
@@ -0,0 +1,25 @@
package com.codeborne.selenide;

/**
* Authentication schemes.
*
* @see <a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Authentication#Authentication_schemes">Web HTTP reference</a>
*/
public enum AuthenticationType {
BASIC("Basic"),
BEARER("Bearer"),
DIGEST("Digest"),
HOBA("HOBA"),
MUTUAL("Mutual"),
AWS4_HMAC_SHA256("AWS4-HMAC-SHA256");

private String value;

This comment has been minimized.

Copy link
@asolntsev

asolntsev Aug 23, 2018

Contributor

I would add private final here.

This comment has been minimized.

Copy link
@rosolko

rosolko Aug 24, 2018

Author Collaborator

@asolntsev Good catch!


AuthenticationType(final String value) {

This comment has been minimized.

Copy link
@asolntsev

asolntsev Aug 23, 2018

Contributor

I would remove final because it does't add any value.

this.value = value;
}

public String getValue() {
return value;
}
}
@@ -0,0 +1,34 @@
package com.codeborne.selenide;

import java.util.Base64;

import static java.nio.charset.StandardCharsets.UTF_8;

final class Credentials {
private String login;
private String password;

Credentials(final String login, final String password) {
this.login = login;
this.password = password;
}

/**
* The resulting string is base64 encoded (YWxhZGRpbjpvcGVuc2VzYW1l).
*
* @return encoded string
*/
String encode() {
final byte[] credentialsBytes = combine().getBytes(UTF_8);
return Base64.getEncoder().encodeToString(credentialsBytes);
}

/**
* Combine credentials with a colon (login:password).
*
* @return combined credentials
*/

This comment has been minimized.

Copy link
@asolntsev

asolntsev Aug 23, 2018

Contributor

Do we really need javadoc for private methods?

This comment has been minimized.

Copy link
@rosolko

rosolko Aug 24, 2018

Author Collaborator

@asolntsev
Yes we do.
The main point thats this object is public and method like this (I mean do some combining logic with special format) is necessary to de described in any way.

This comment has been minimized.

Copy link
@asolntsev

asolntsev Aug 24, 2018

Contributor

Let's make it public when it's really needed. Currently it's not used, so there is no reason to make it public.

This comment has been minimized.

Copy link
@rosolko

rosolko Aug 24, 2018

Author Collaborator

Agree. Just remove javadoc since it's specially private method.

private String combine() {
return String.format("%s:%s", login, password);
}
}
@@ -9,6 +9,7 @@
import com.codeborne.selenide.impl.SelenideFieldDecorator;
import com.codeborne.selenide.impl.WebElementsCollectionWrapper;

import net.lightbody.bmp.BrowserMobProxy;
import org.openqa.selenium.Alert;
import org.openqa.selenium.By;
import org.openqa.selenium.JavascriptExecutor;
@@ -38,6 +39,8 @@
import static com.codeborne.selenide.Configuration.pollingInterval;
import static com.codeborne.selenide.Configuration.timeout;
import static com.codeborne.selenide.WebDriverRunner.closeWebDriver;
import static com.codeborne.selenide.WebDriverRunner.getAndCheckWebDriver;
import static com.codeborne.selenide.WebDriverRunner.getSelenideProxy;
import static com.codeborne.selenide.WebDriverRunner.getWebDriver;
import static com.codeborne.selenide.WebDriverRunner.hasWebDriverStarted;
import static com.codeborne.selenide.WebDriverRunner.supportsJavascript;
@@ -101,6 +104,35 @@ public static void open(String relativeOrAbsoluteUrl, String domain, String logi
mockModalDialogs();
}

/**
*
* The main starting point in your tests.
* <p>
* Open browser and pass authentication using build-in proxy.
* <p>
* A common authenticationType is "Basic". See Web HTTP reference for other types.
* <p>
* {@code Configuration.fileDownload == Configuration.FileDownloadMode.PROXY;} - is required configuration.

This comment has been minimized.

Copy link
@asolntsev

asolntsev Aug 23, 2018

Contributor

I would rephrase it to something like "this method can only work if {@code Configuration.fileDownload == Configuration.FileDownloadMode.PROXY;}"

This comment has been minimized.

Copy link
@rosolko

rosolko Aug 24, 2018

Author Collaborator

@asolntsev Understood. Thanks!

*
* @param relativeOrAbsoluteUrl
* @param authenticationType
* @param login
* @param password

This comment has been minimized.

Copy link
@asolntsev

asolntsev Aug 23, 2018

Contributor

I would remove all useless lines from javadoc, like @param login without description.

This comment has been minimized.

Copy link
@rosolko

rosolko Aug 24, 2018

Author Collaborator

@asolntsev
We already had such empty javadocs lines for old open(relativeOrAbsoluteUrl, domain, login, password) method.
I do it in one style during new implementation.
In this case we had to reformat all methods related to this point.

This comment has been minimized.

Copy link
@asolntsev

asolntsev Aug 24, 2018

Contributor

I agree, we can cleanup javadoc later.

*
* @see <a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Proxy-Authorization">Web HTTP reference</a>
* @see AuthenticationType
*/
public static void open(final String relativeOrAbsoluteUrl, final AuthenticationType authenticationType,
final String login, final String password) {
getAndCheckWebDriver();

This comment has been minimized.

Copy link
@asolntsev

asolntsev Aug 22, 2018

Contributor

Here is too much logic for Selenide class. All this stuff should be in SelenideProxyServer and/or navigator.open. In this case we would not need to increase ClassFanOutComplexity in checkstyle.xml

final BrowserMobProxy proxy = getSelenideProxy().getProxy();

This comment has been minimized.

Copy link
@asolntsev

asolntsev Aug 22, 2018

Contributor

It's not a good idea to use BrowserMobProxy directly. Much better would be to add RequestFilter using getSelenideProxy().addRequestFilter(). This filter could add required headers.

final Credentials credentials = new Credentials(login, password);
final String authorization = String.format("%s %s", authenticationType.getValue(), credentials.encode());

This comment has been minimized.

Copy link
@asolntsev

asolntsev Aug 22, 2018

Contributor

I personally do not like final near every local variable and method parameter. It's a noise.

proxy.addHeader("Authorization", authorization);
proxy.addHeader("Proxy-Authorization", authorization);
navigator.open(relativeOrAbsoluteUrl);
}

/**
* @see Selenide#open(URL, String, String, String)
*/
@@ -1,5 +1,8 @@
package integration;

import com.codeborne.selenide.AuthenticationType;
import com.codeborne.selenide.Configuration;
import com.codeborne.selenide.Configuration.FileDownloadMode;
import org.junit.jupiter.api.Test;

import static com.codeborne.selenide.Condition.text;
@@ -12,4 +15,11 @@ void canPassBasicAuth() {
open("/basic-auth/hello", "", "scott", "tiger");
$("body").shouldHave(text("Hello, scott:tiger!"));
}

@Test
void canAuthUsingProxy() {
Configuration.fileDownload = FileDownloadMode.PROXY;
open("/basic-auth/hello", AuthenticationType.BASIC, "scott", "tiger");
$("body").shouldHave(text("Hello, scott:tiger!"));
}
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.