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

Login window instead of WWW-Authenticate #1301

Merged
merged 2 commits into from
Feb 5, 2018

Conversation

whisperity
Copy link
Contributor

@whisperity whisperity commented Jan 17, 2018

Closes #1220.

Instead of prompting the user for WWW-Authenticate (the normal browser popup window authentication), use a facade put together in Dojo to ask for user credentials. This way, it is the Thrift API that handles the login of the user, the same way the command-line client does it.

Browsers cache the credentials of a Authenticate header which is very hacky to remove. Thanks to dropping that whole functionality, we can now introduce a Log out feature, which this patch also does.

@whisperity whisperity added enhancement 🌟 RDY-OnHold 🛑 Patch reviewed and ready, but don't merge due to having to merge a dependent patch first. GUI 🎨 labels Jan 17, 2018
@whisperity whisperity removed the RDY-OnHold 🛑 Patch reviewed and ready, but don't merge due to having to merge a dependent patch first. label Jan 18, 2018
font-size: 12pt;
display: inline-block;
margin-bottom: 18px;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

No new line at the end of file.

document.body.appendChild(layout.domNode);
layout.startup();
};
});
Copy link
Contributor

Choose a reason for hiding this comment

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

No new line at the end of this file.

www/login.html Outdated
<!-- CSS -->

<link type="text/css" rel="stylesheet" href="scripts/plugins/dojo/dijit/themes/claro/claro.css" />
<link type="text/css" rel="stylesheet" href="scripts/plugins/dojo/dojox/form/resources/CheckedMultiSelect.css"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Do not import CheckedMultiSelect, claroGrid, jsPlumb, marked.min. We do not use these at the login page.


this.btnSubmit = new Button({
label : "Login",
onClick : function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be great to fire this event by pressing ENTER button.

@csordasmarton
Copy link
Contributor

Login in and after log out from the product page will result a blank page.

@whisperity
Copy link
Contributor Author

Rebased TOT for the minor refactoring work introduced in #1172.


product_endpoint, path = routing.split_client_GET_request(self.path)

if self.server.manager.isEnabled() and not auth_session \
Copy link
Contributor

Choose a reason for hiding this comment

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

SessionManager instance has no attribute 'isEnabled'

Use is_enabled property instead of this.

this.btnSubmit = new Button({
label : "Login",
onClick : function () {
that._doLogin();
Copy link
Contributor

Choose a reason for hiding this comment

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

If the login has failed, do not clear the username field and put the focus on the password input field.

Copy link
Contributor

Choose a reason for hiding this comment

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

If I click on the login button the authentication takes too much time. It would be great to use a dojo stand by (https://dojotoolkit.org/reference-guide/1.10/dojox/widget/Standby.html) to show that the operation is in process.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

First comment fixed.

Second I will look into later, no time to do right now. Also, should the authentication really be that slow?

region : 'center',
postCreate : function () {
var smallerContainer = domConstruct.create('div', {
class : 'login-form'
Copy link
Contributor

Choose a reason for hiding this comment

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

Use id instead of class

@@ -0,0 +1,15 @@
.login-form {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use this:

#login-form {
  display: block;
  position: relative;
  top: 25%;
  margin: 0 auto;
  width: 20%;
  background-color: #edf4fa;
  border: 1px solid;
  border-color: #e5e6e9 #dfe0e4 #d0d1d5;
  border-radius: 5px;
}

#login-form .mbox.mbox-error {
  background-color: white;
}

#login-form .login-prompt {
  font-weight: bold;
  font-size: 12pt;
  display: inline-block;
  margin-bottom: 18px;
}

#login-form .formElement .form-input {
    width: 100%;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You sure we want it to look like this?

image

The prompt for access is getting wrapped. If the window is small, this happens:

image

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 use the min-width css property to set a minimal width for the login form.


var that = this;
function keypressHandler(evt) {
if (evt.keyCode === keys.ENTER) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Problem on login:

  • Type the username
  • Type the password
  • Hit enter

The login failed because the password wasn't set properly:

[1, "performLogin", 1, 0, {1: {str: "Username:Password"}, 2: {str: "test:"}}]

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.

<link type="text/css" rel="stylesheet" href="scripts/plugins/dojo/dijit/themes/claro/claro.css" />
<link type="text/css" rel="stylesheet" href="style/codecheckerviewer.css" />
<link type="text/css" rel="stylesheet" href="style/login.css" />
<link type="text/css" rel="stylesheet" href="style/productlist.css" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does the login use the productlist css?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The styles for formElement and form-input are defined in the productlist.css file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants