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

API js - use base64encode which can deal with UTF8 passwords #3682

Merged
merged 3 commits into from
Mar 27, 2018
Merged

API js - use base64encode which can deal with UTF8 passwords #3682

merged 3 commits into from
Mar 27, 2018

Conversation

himdel
Copy link
Contributor

@himdel himdel commented Mar 26, 2018

This adds 2 npm packages:

  • base64js - JS implementation of base64 which can deal with arbitrary encodings
  • text-encoder-lite - a shim for the TextEncoder api, useable for utf8 strings (even in IE)

and replaces the API.login use of window.btoa with a custom base64encode function implemented using the 2 libraries.

This way, users can use passwords with unicode chars.

Note: on modern browsers, this will handle any unicode password, including emoji.
On IE, more modern unicode versions may not be fully supported (czech or japanese chars should be fine, emoji may not).

The difference:

btoa('babicka')
"YmFiaWNrYQ=="

base64encode("babicka")
"YmFiaWNrYQ=="

btoa('babička')
VM7832:1 Uncaught DOMException: Failed to execute 'btoa' on 'Window': The string to be encoded contains characters outside of the Latin1 range.

base64encode("babička")
"YmFiacSNa2E="

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1527316

can't really assume the login screen will depend only on old JS anymore
`window.btoa("sněhulák")` fails on InvalidCharacterError: String contains an invalid character

because it only expects latin1 chars

We need to base64 encode the login:password pair even when the password uses non-latin1 chars :)

Implementation adapted from https://developer.mozilla.org/en-US/docs/Web/API/WindowBase64/Base64_encoding_and_decoding#Solution_2_%E2%80%93_rewrite_the_DOMs_atob()_and_btoa()_using_JavaScript's_TypedArrays_and_UTF-8

https://bugzilla.redhat.com/show_bug.cgi?id=1527316
@himdel
Copy link
Contributor Author

himdel commented Mar 26, 2018

Testing:

add a user with special (unicode) characters in the password
try to log in

@miq-bot
Copy link
Member

miq-bot commented Mar 26, 2018

Checked commits https://github.com/himdel/manageiq-ui-classic/compare/af3c209964ca45c8cf4297b909383e2ab0e5e155~...6a0688bdc591be091cbb0cd822c9e827a9031745 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 🍰

@mzazrivec mzazrivec self-assigned this Mar 27, 2018
@mzazrivec mzazrivec added this to the Sprint 83 Ending Apr 9, 2018 milestone Mar 27, 2018
@mzazrivec mzazrivec merged commit aa6b2fd into ManageIQ:master Mar 27, 2018
@himdel himdel deleted the password-utf-bz1527316 branch March 27, 2018 11:23
@himdel
Copy link
Contributor Author

himdel commented Mar 27, 2018

gaprindashvili: Please backport together with #3687, which fixes a bug in older browsers

@simaishi
Copy link
Contributor

simaishi commented Apr 2, 2018

@himdel Can this (and related PRs) be fine/yes?

simaishi pushed a commit that referenced this pull request Apr 2, 2018
API js - use base64encode which can deal with UTF8 passwords
(cherry picked from commit aa6b2fd)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1562797
@simaishi
Copy link
Contributor

simaishi commented Apr 2, 2018

Gaprindashvili backport details:

$ git log -1
commit e6aeca117b6469b09271630850da8c81b11d3b7e
Author: Milan Zázrivec <mzazrivec@redhat.com>
Date:   Tue Mar 27 09:44:26 2018 +0200

    Merge pull request #3682 from himdel/password-utf-bz1527316
    
    API js - use base64encode which can deal with UTF8 passwords
    (cherry picked from commit aa6b2fdc7b62facac0d6d03616d482eef307f010)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1562797

@himdel
Copy link
Contributor Author

himdel commented Apr 3, 2018

@simaishi They can't, I think I need to create a separate PR - no webpack in fine.

On it.. :)

@himdel
Copy link
Contributor Author

himdel commented Apr 3, 2018

Fine version created in #3711

@simaishi
Copy link
Contributor

Backported to Fine via #3711

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.

None yet

4 participants