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

RFC: Improve login #2471

Open
donker opened this issue Nov 11, 2018 · 35 comments
Open

RFC: Improve login #2471

donker opened this issue Nov 11, 2018 · 35 comments

Comments

@donker
Copy link
Contributor

donker commented Nov 11, 2018

Description of problem

The login experience has remained untouched for years and shows its age:

  1. It takes forever to load which is especially noticeable when you use the popup
  2. When clicking login the form sits there while the user is being logged in - the user should see some sort of spinner
  3. It's not responsive (issue RFC: Mobile Friendly Login, Enhance Existing or New Module #2469)
  4. The "keep me logged in" checkbox is below the login/cancel buttons - this should be above
  5. It is difficult to style/adapt given the nr of fixed classes - it should be templateable

Description of solution

To attack problems nr 1 and nr 2 we could look at preloading a login screen. The complexity is that your page may be http and you want the login to be https. But we could consider having an improved experience only for sites that implement https and then go through a web api call to log in. This would allow us to do all the user feedback at the front end which makes things simpler.

To attack 3-5 we need to look at making this templateable. The complexity is that this control is extensible and auth providers can hook in. But IMHO this shouldn't trump us moving forward to improve this. One solution I've had some luck with is the use of a razor template inside the skin folder which describes how the login panel looks including the optional components.

@kurtwilbies
Copy link

@donker
Can you share your (customized) solution. I actually hope this problem will be solved in 9.3. Login is the hart of each (secure) app.

@jeremy-farrance
Copy link
Contributor

Please also -

  1. we need to have the option to unhook this 'login thing' from targeting the current page's Skin and ContentPane. Maybe there should be some settings at the Site (Portal) level that provide some choices about how the Event (user suddenly needs to authenticate) is handled and routed. What I am trying to say is it needs to uncouple from the current skin/pane logic and become its own thing with an OPTION (host and site level) to still happen the old way (which could/should be the natural default for the next few versions). The new, default end result needs to be simple and generic and work with the existing providers.

  2. please do not forget being able to login without a mouse. The tab order moving between fields needs to be logical and pressing enter needs to submit.

@valadas
Copy link
Contributor

valadas commented Nov 13, 2018

I remember on older dnn versions, you had an option of enabling or disabling popups. With popups enable it would center the dialog on the screen. Was that intentionally remove or was it just a forgotten option in the Persona Bar site settings?

@thabaum
Copy link
Contributor

thabaum commented Nov 21, 2018

It would be nice to have a two part authentication option that sends a message to email with pin to log in if enabled in host and in user account for added security option. Possibility for SMS messaging as well. The email sending a pin I think would be a more basic and simple way to add one more level of security.

Option for Google reCaptcha that uses the checkbox style checker for registration or login if set.

Common popular authentication providers working with latest logos and login buttons that work with theme.

@moorecreative
Copy link

++ on the comment for having 2-part authentication as an option, that would be a great feature!

@kurtwilbies
Copy link

2-part auth is nowadays a must. My vote.

@mitchelsellers
Copy link
Contributor

Any two factor implementation should be provider, or interface driven so that new methods can be implemented. For example SMS which isn’t practical without other config etc.

@sleupold
Copy link
Contributor

IMO, points 1 to 5 are nobrainers and should be fixed

@MaiklT
Copy link
Contributor

MaiklT commented Jun 12, 2019

Please please please add 2-factor authentication (e.g. using Google Authenticator)

@valadas valadas added this to the 10.0.0 milestone Jul 28, 2019
@stale
Copy link

stale bot commented Oct 26, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 26, 2019
@thabaum
Copy link
Contributor

thabaum commented Oct 27, 2019

Hello stale bot, I would like to bring this back to life. 2 Factor Authentication would be a nice touch to have soon for security reasons.

@stale stale bot removed the stale label Oct 27, 2019
@thabaum
Copy link
Contributor

thabaum commented Oct 27, 2019

IMO, points 1 to 5 are nobrainers and should be fixed

I agree.

@thabaum
Copy link
Contributor

thabaum commented Oct 31, 2019

Another security feature is to send a new simple text verification code to a user via email when they log in using a browser for the first time that is unrecognized being used to log in previously. Register browsers in DNN that have been verified so it only happens first time a browser gets setup for being accepted for authentication.

@thabaum
Copy link
Contributor

thabaum commented Nov 6, 2019

#3252

I created another issue for Two-Factor Authentication which if I can come up with a solution before someone else jumps on it I will work with email two-factor to start.

1-5 here I might be able to knock out along the way. But I might get stuck hoping to fight through these as this is part of my own goals to improve CE or ask for them to get improved. Seems like some things can get done faster if you do them yourself. It looks like 5 separate issues that need to each be handled independently but relate to the login UI. I will be looking at these files so I will try to attempt some PR's for them.

Issues # 1 + 2 looks like maybe HTTP/2 might help it out in the future? A spinner would help entertain.

I see an order like this:

  • Username (Email if set for Site Settings)
  • Password
  • Two-Factor (Send Request, Enter Code)
  • Captcha
  • Remember (Issue # 4)
  • Login
  • Register + Reset Password Controls

  • Other providers

Issues # 3 and # 5 seem like they could relate as well. It would be nice to see it layout as a bootstrap designed theme which is something I could also look into, that use font awesome icons get a boost in look here if the theme used supports it.

@valadas
Copy link
Contributor

valadas commented Nov 8, 2019

I would like it to be responsive by itself without relying on a css framework. Themers can restyle that per their requirements but we need some default that works responsively no mater what theme if not otherwise styled. I have some ideas for this but no time to put this in before Dnn10 for sure.

mitchelsellers pushed a commit that referenced this issue Nov 13, 2019
 (#3256)

* Move Remember Login Above Login Button

* Update Login.ascx

* file copy paste issue...
donker pushed a commit that referenced this issue Nov 15, 2019
* Do not store these portal settings when setting their property on the portalsettings object

* Removed DnnHttpControllerActivator and added DependencyResolver implementation using the IServiceProvider

* Added xml comments for the new DnnDependencyResolver

* Added DnnDependencyResolver Unit Tests

* Replaced IServiceProvider mock with full IServiceProvider implementation to work with Web API tests

* Added more unit tests for the DnnDependencyResolver to cover GetServices (multiple) and BeginScope

* Do not show "No Search results" message when no search is performed. (#3203)

* Revision of build process part 1 (#3137)

* Move website folder

* repairs

* Update Cake

* Move external task to its own file

* Begin packaging scripts in cake

* Further work on packaging

* Ensure Telerik project also gets new version nr

* Move Telerik PDF to its proper place

* New logic for packages that are already in final form in the repo and just need to be zipped.

* Bring fips Lucene lib back under git control

* Newtonsoft package and repair to telerik package

* Splitting off other tasks

* Upgrade and deploy packages

* Symbols package

* Get rid of platform ver in manifest

* Final fixes - working packages

* Move compilation stuff

* House cleaning

* peg utils version

* Fix to ckep script

* suppress warnings

* Variable initialization issue

* Removing unused scripts

* Revert change to Telerik package version

* Move "other" to "thirdparty"

* Get rid of hard coded Newtonsoft version

* Max cpu to 4 for compiling

* Repairs to MSBuild scripts to get AE to build properly

* Correcting paths

* Cleaning up

* Trying to fix ignore file

* Update Build/Cake/thirdparty.json

Co-Authored-By: Brian Dukes <bdukes@engagesoftware.com>

* Remove absolute path from .gitignore

* Fix extension of DNC library package

* Merging AE and DNN build scripts

* Fixes to scripts

* Clean up symbols project for AE since they are included in DNN project now

* Add previously untracked Yahoo compressor dlls

* Remove version information from SolutionInfo.cs

* Fix errors introduced in rebase

* Fix incorrect paths

* Change build order of MSBuild to build extensions before personabar ui.

* Make white-space visible in log viewer (#3198)

* DNN-34236: check the url with case insensitive.

* DNN-29417 Username is changed to email address in the user profile if the "Use Email Address as Username" is enabled.

* Fix to installer and the dependencies of the HTML and DA modules

* DNN-24945 - Always redirect to the source portal page after creating (#3223)

group

* Fix Typo (#3234)

* Improve querystring parse (#3242)

* Improve props popup

* Improve input checking of sites (#3243)

* Fixes an upgrade issue that affects 9.4.1 => 9.4.2 upgrades (#3282)

* Fixes an issue with a duplication of assembly binding

* Removed hardcode version in manifest

* Further reorganization of build process (#3236)

* Introduce settings and add custom version nr

* Ability to create a local dev site

* Move node workspaces to root

* Reshuffling of variables in MSBuild

* Repairs

* Fix

* Allow local override of global build variables

* Only save manifests if the file is not there. This prevents overwriting in case of a failed build before.

* Ensure projects build in debug to the website path specified by the platform build settings

* Don't track vscode folder

* Fixes #3168 Test Sending Email Issues (#3237)

* Use Admin Email for Send if smtpHostMode false

* Fix Test Email Settings TO and FROM Addresses

If I did this right and it should make sense is I set the test email to be able to go to the user testing.  After all they are who are trying to make it work.  It can be any email used by anyone and usually to see this feature you are a host or portal administrator so security should not be as much of a concern here.

The FROM address will use the HOST email settings if set to HOST mode in DNN for the portal, otherwise it should use the Portal Administrators Email account to send from.

This however does not address the issue of using the email address supplied in settings and creating another setting which will be applied next to figure out the correct logic.

* Update ServerSettingsSmtpAdminController.cs

* Fix build issue to previous changes.

PortalSettings.UserInfo.Email 
is this the current user email?

I am still getting my things setup.  I ran VS 2019 to build successfully and I am trying to read through the namespaces correctly so I can understand the changes.  I noticed a number of files possibly needing updates due to being deprecated in 9.4.2 but after I changed the namespace issue those warnings went away.  

One last thought is how do you get intellisense working on DNN solution.  Is that a dream to have or am I missing something in my setup?

* Change to address to the current user's email testing the server.

I believe a UI to set the TO address would be nice to help troubleshoot and address issues trying to send to a specific user.

* Corrected To address for current user email

* TO Current User Email

* DNN-29110 Site Assets > Select All is not working (#3251)

* DNN-29110 Site Assets > Select All is not working

* Code review fix

* Moves Remember Login above Login Button:  referenced in issue #2471 #3255 (#3256)

* Move Remember Login Above Login Button

* Update Login.ascx

* file copy paste issue...

* DNN-34250 Search is not working even after re-Indexing Search for All DNN Portals (#3260)

* Fixes duplication in DotNetNuke.Website.csproj

* Removed reference to missing LeftMenu files
@valadas
Copy link
Contributor

valadas commented Nov 18, 2019

This RFC as now been here for a year, it looks like it would be multiple smaller actionable items to implement right? Can people who want to pickup some individual items create an issue for that specific part and we close this RFC let's say in a couple weeks so we don't spread too much in discussions in a too broad scoped issue?

I for one am working on some new reusable components while building the new file manager and as part of that will have a lightweight modal component that I would like to reuse for the login/register and other modals in Dnn. This will be about a 2Kb component with no dependencies as opposed to now that we need jQuery, jQueryUI, dnn.jquery and so on that weights about 1MB uncompressed but still about 400Kb compressed, which is huge when all you need is a modal to login. This should help a lot for the mentioned delay... I did not create an issue specifically for this but will do after the file manager and apply that where we can.

valadas pushed a commit that referenced this issue Dec 3, 2019
* Do not store these portal settings when setting their property on the portalsettings object

* Removed DnnHttpControllerActivator and added DependencyResolver implementation using the IServiceProvider

* Added xml comments for the new DnnDependencyResolver

* Added DnnDependencyResolver Unit Tests

* Replaced IServiceProvider mock with full IServiceProvider implementation to work with Web API tests

* Added more unit tests for the DnnDependencyResolver to cover GetServices (multiple) and BeginScope

* Do not show "No Search results" message when no search is performed. (#3203)

* Revision of build process part 1 (#3137)

* Move website folder

* repairs

* Update Cake

* Move external task to its own file

* Begin packaging scripts in cake

* Further work on packaging

* Ensure Telerik project also gets new version nr

* Move Telerik PDF to its proper place

* New logic for packages that are already in final form in the repo and just need to be zipped.

* Bring fips Lucene lib back under git control

* Newtonsoft package and repair to telerik package

* Splitting off other tasks

* Upgrade and deploy packages

* Symbols package

* Get rid of platform ver in manifest

* Final fixes - working packages

* Move compilation stuff

* House cleaning

* peg utils version

* Fix to ckep script

* suppress warnings

* Variable initialization issue

* Removing unused scripts

* Revert change to Telerik package version

* Move "other" to "thirdparty"

* Get rid of hard coded Newtonsoft version

* Max cpu to 4 for compiling

* Repairs to MSBuild scripts to get AE to build properly

* Correcting paths

* Cleaning up

* Trying to fix ignore file

* Update Build/Cake/thirdparty.json

Co-Authored-By: Brian Dukes <bdukes@engagesoftware.com>

* Remove absolute path from .gitignore

* Fix extension of DNC library package

* Merging AE and DNN build scripts

* Fixes to scripts

* Clean up symbols project for AE since they are included in DNN project now

* Add previously untracked Yahoo compressor dlls

* Remove version information from SolutionInfo.cs

* Fix errors introduced in rebase

* Fix incorrect paths

* Change build order of MSBuild to build extensions before personabar ui.

* Make white-space visible in log viewer (#3198)

* DNN-34236: check the url with case insensitive.

* DNN-29417 Username is changed to email address in the user profile if the "Use Email Address as Username" is enabled.

* Fix to installer and the dependencies of the HTML and DA modules

* DNN-24945 - Always redirect to the source portal page after creating (#3223)

group

* Fix Typo (#3234)

* Improve querystring parse (#3242)

* Improve props popup

* Improve input checking of sites (#3243)

* Fixes an upgrade issue that affects 9.4.1 => 9.4.2 upgrades (#3282)

* Fixes an issue with a duplication of assembly binding

* Removed hardcode version in manifest

* Further reorganization of build process (#3236)

* Introduce settings and add custom version nr

* Ability to create a local dev site

* Move node workspaces to root

* Reshuffling of variables in MSBuild

* Repairs

* Fix

* Allow local override of global build variables

* Only save manifests if the file is not there. This prevents overwriting in case of a failed build before.

* Ensure projects build in debug to the website path specified by the platform build settings

* Don't track vscode folder

* Fixes #3168 Test Sending Email Issues (#3237)

* Use Admin Email for Send if smtpHostMode false

* Fix Test Email Settings TO and FROM Addresses

If I did this right and it should make sense is I set the test email to be able to go to the user testing.  After all they are who are trying to make it work.  It can be any email used by anyone and usually to see this feature you are a host or portal administrator so security should not be as much of a concern here.

The FROM address will use the HOST email settings if set to HOST mode in DNN for the portal, otherwise it should use the Portal Administrators Email account to send from.

This however does not address the issue of using the email address supplied in settings and creating another setting which will be applied next to figure out the correct logic.

* Update ServerSettingsSmtpAdminController.cs

* Fix build issue to previous changes.

PortalSettings.UserInfo.Email 
is this the current user email?

I am still getting my things setup.  I ran VS 2019 to build successfully and I am trying to read through the namespaces correctly so I can understand the changes.  I noticed a number of files possibly needing updates due to being deprecated in 9.4.2 but after I changed the namespace issue those warnings went away.  

One last thought is how do you get intellisense working on DNN solution.  Is that a dream to have or am I missing something in my setup?

* Change to address to the current user's email testing the server.

I believe a UI to set the TO address would be nice to help troubleshoot and address issues trying to send to a specific user.

* Corrected To address for current user email

* TO Current User Email

* DNN-29110 Site Assets > Select All is not working (#3251)

* DNN-29110 Site Assets > Select All is not working

* Code review fix

* Moves Remember Login above Login Button:  referenced in issue #2471 #3255 (#3256)

* Move Remember Login Above Login Button

* Update Login.ascx

* file copy paste issue...

* DNN-34250 Search is not working even after re-Indexing Search for All DNN Portals (#3260)

* DNN-32474 - Recursive delete option is added (#3249)

* Move Country Above Region in UserProfile.cs

* spacing issue resolved.

* I keep saving but github is moving it on its own.

* Fixed tab for spaces to resolve spacing issues

* Add PortalSettings overloads back (#3295)

Fixes #3289

* Updates issue templates as per 9.4.2 release

* Move Email Above Username in User.ascx #3305 (#3306)

* Move Email Above Username in User.ascx

* Fixed tab index values in User.ascx

* Fix NuGet warning NU5048 (#3304)

The iconUrl property of the nuspec file is deprecated, it's recommended
to add an icon property, which points to a file within the package,
while also retaining the iconUrl property for older clients.
See https://docs.microsoft.com/en-us/nuget/reference/nuspec#iconurl

* Word-Break added to Journal P (#3307)

* Set core object versions to core version (#3287)

Ensure skin and radeditorprovider get the core version nr and add SPROC to set the version of all core packages/desktopmodules to the core version

Fixes #3239

* Third installment of build reorganization (#3285)

* Improve versioning

* New approach: keep versions in source control and change upon CI build

* Remove previous versioning logic

* Cleaning up tasks

* Fixes

* Ensure UpdateDnnManifests targets just the right manifests

* Harmonize naming of zips and nuget packages

* Testing webpack building to dev website

* Adjust webpack projects in AE to build to dev site

* Fixes

* Delete duplicate stuff

* This is generated by Webpack and shouldn't be tracked

* Include missing project from Lerna

* Fixes for building SitesListView

* Update to packages

* Further cleaning

* Remove RadEditorProvider

* Fix ModuleSettings > Added to Pages grid, pager wasn't working

* Allow only alphanumeric characters to be used when forming url from website name (#3316)

* Ensure UpdateDnnManifests doesn't run for building dev site (#3314)

* Reload page when the code tells you to (#3315)

* Reload page when the code tells you to

* Optimize code

* Google Analytics TrackingID is stored in lowercase #3318 (#3322)

* Update sums.resources

* Fix casing mismatch errors in security analyzer file compare

* Update Dnn.AdminExperience/Extensions/Content/Dnn.PersonaBar.Extensions/Components/Security/Checks/CheckDefaultPage.cs

Co-Authored-By: Brian Dukes <bdukes@engagesoftware.com>

* Registered IPersonaBarContainer with DI (#3329)

* Document approval process and group

* Fix typo

* Relaxed nuget.cake wildcard (#3331)
@mitchelsellers
Copy link
Contributor

Recaptcha support, from a core perspective is going to be really hard. Especially giving the complexity/cost in setup of recaptcha.

I will discuss with the rest of the approval team next week to discuss if it is a consideration.

@jeremy-farrance
Copy link
Contributor

I have seen a lot of advancements in login ideas in recent years. Maybe this gets easier with steps; a pipeline or workflow. I really, really like sites that have recently separated things; you start on a pre-login page with CAPTCHA and your username/email only, Then if that lets you through you end up on the authentication page where you enter just your password. This "seems" to reduce complexity somewhat, but I also realize it may not fit with what exists now and may even be more work. But I felt it was worth mentioning. "With the right semantics and partitioning, anything can be simplified." - nobody

@stale
Copy link

stale bot commented Jan 21, 2021

We have detected this issue has not had any activity during the last 90 days. That could mean this issue is no longer relevant and/or nobody has found the necessary time to address the issue. We are trying to keep the list of open issues limited to those issues that are relevant to the majority and to close the ones that have become 'stale' (inactive). If no further activity is detected within the next 14 days, the issue will be closed automatically.
If new comments are are posted and/or a solution (pull request) is submitted for review that references this issue, the issue will not be closed. Closed issues can be reopened at any time in the future. Please remember those participating in this open source project are volunteers trying to help others and creating a better DNN Platform for all. Thank you for your continued involvement and contributions!

@stale stale bot added the stale label Jan 21, 2021
@MaiklT
Copy link
Contributor

MaiklT commented Jan 23, 2021

Still an issue.

@stale stale bot removed the stale label Jan 23, 2021
@stale stale bot added the stale label Jun 2, 2021
@MaiklT
Copy link
Contributor

MaiklT commented Jun 6, 2021

Still an issue

@MaiklT
Copy link
Contributor

MaiklT commented Sep 5, 2021

Still an issue

@stale stale bot removed the stale label Sep 5, 2021
@stale stale bot added the stale label Jan 9, 2022
@moorecreative
Copy link

may be old, but not stale. refresh please dependabot

@stale stale bot removed the stale label Feb 7, 2022
@stale stale bot added the stale label Jul 31, 2022
@dnnsoftware dnnsoftware deleted a comment from stale bot Jul 31, 2022
@dnnsoftware dnnsoftware deleted a comment from stale bot Jul 31, 2022
@dnnsoftware dnnsoftware deleted a comment from stale bot Jul 31, 2022
@dnnsoftware dnnsoftware deleted a comment from stale bot Jul 31, 2022
@stale stale bot removed stale labels Jul 31, 2022
@stale
Copy link

stale bot commented Nov 2, 2022

We have detected this issue has not had any activity during the last 90 days. That could mean this issue is no longer relevant and/or nobody has found the necessary time to address the issue. We are trying to keep the list of open issues limited to those issues that are relevant to the majority and to close the ones that have become 'stale' (inactive). If no further activity is detected within the next 14 days, the issue will be closed automatically.
If new comments are are posted and/or a solution (pull request) is submitted for review that references this issue, the issue will not be closed. Closed issues can be reopened at any time in the future. Please remember those participating in this open source project are volunteers trying to help others and creating a better DNN Platform for all. Thank you for your continued involvement and contributions!

@stale stale bot added the stale label Nov 2, 2022
@rodrigoratan
Copy link
Contributor

still important!

@stale stale bot removed the stale label Nov 2, 2022
@valadas
Copy link
Contributor

valadas commented Nov 2, 2022

Yeah, but apparently not enough for anybody to tackle this, this issue is 4 years old, does anybody intend to tackle this ?

@stale
Copy link

stale bot commented Jun 18, 2023

We have detected this issue has not had any activity during the last 90 days. That could mean this issue is no longer relevant and/or nobody has found the necessary time to address the issue. We are trying to keep the list of open issues limited to those issues that are relevant to the majority and to close the ones that have become 'stale' (inactive). If no further activity is detected within the next 14 days, the issue will be closed automatically.
If new comments are are posted and/or a solution (pull request) is submitted for review that references this issue, the issue will not be closed. Closed issues can be reopened at any time in the future. Please remember those participating in this open source project are volunteers trying to help others and creating a better DNN Platform for all. Thank you for your continued involvement and contributions!

@stale stale bot added the stale label Jun 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests