AuthenticationAPI - use base64encode which can deal with UTF8 passwords #1411

himdel · 2018-03-26T16:51:50Z

This is the same change as ManageIQ/manageiq-ui-classic#3682, except for SUI.

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 AuthenticationAPI use of $base64.encode 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

suffers from the same problem as window.btoa - can't handle unicode strings https://bugzilla.redhat.com/show_bug.cgi?id=1527316

himdel · 2018-03-26T16:52:34Z

Testing:

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

`window.btoa("sněhulák")` fails on InvalidCharacterError: String contains an invalid character because it only expects latin1 chars `$base64.encode` from `angular-base64` has the same problem. 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

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

AllenBW

Jumped in, checked it out, café is now a valid password!

LG2M Soon as travis is made happy (style things) will merge this in!

AllenBW · 2018-03-26T20:11:56Z

oh my, now those are annoying failures... lemme see what can be done to help out travis... 🤔

himdel · 2018-03-26T22:01:15Z

So.. this does fail with the PhantomJS launcher but not with Chrome launcher .. also, looks like this kind of import is not really used in SUI, almost everything exernal is required in client/app.js.

Also also, the specs kinda don't even run... looks like a runtime error early on, before any of the real code gets run (otherwise we'd see the promise either resolve, or catch, but if I add a .then(done, done), it still times out).

That leads me to think this could be some webpack config error (which we could see as not being able to login in older browsers) or a difference in how the specs are compiled (babel vs webpack?).

Either way, I'll try fixing in the morning by moving that import to client/app.js, and seeing if that works :).

himdel · 2018-03-27T12:34:24Z

Never mind, this (travis) is actually a different problem .. TextEncoder works just fine (in browsers that support it), but TextEncoderLite doesn't seem to have an encode method we rely on.

.. and that's because .. completely logically .. TextEncoderLite actually only exports TextDecoderLite when in module context :D

(problem described more in ManageIQ/manageiq-ui-classic#3687)

… the encoder and explicitly say window.TextEncoder for when it doesn't exist

miq-bot · 2018-03-27T14:11:51Z

Checked commits https://github.com/himdel/manageiq-ui-service/compare/83273df13bd34afc633a07141450307c3c7719dc~...1384c4aeb94b957c62d3dade49ea8edbf8fe6694 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. 🏆

himdel · 2018-03-27T14:11:59Z

@AllenBW looks like this fixes the specs, finally :) PTAL :)

AllenBW · 2018-03-27T14:27:42Z

package.json

@@ -112,6 +112,7 @@
    "sprintf-js": "1.1.1",
    "standard-loader": "6.0.1",
    "style-loader": "0.20.2",
+    "text-encoder-lite": "git://github.com/coolaj86/TextEncoderLite.git#e1e031b",


DANG that's annoying... update the to the version we need... but don't release it, 1.0.2 looks like what we want... but its nowhere cept on the commit...

@himdel yah ok with rewording this as follows? "text-encoder-lite": "coolaj86/TextEncoderLite#e1e031b",

AllenBW

👌 💌 🎉

now everyone can use the password café!!!

AllenBW · 2018-03-28T14:01:14Z

@simaishi gooooood to get back! 👍 🌮

AuthenticationAPI - use base64encode which can deal with UTF8 passwords (cherry picked from commit 0ae3d01) https://bugzilla.redhat.com/show_bug.cgi?id=1562797

simaishi · 2018-04-02T15:29:57Z

Gaprindashvili backport details:

$ git log -1
commit 98c826b38d0982221658923c5f6ee4392294e571
Author: Allen Wight <allen.b.wight@gmail.com>
Date:   Wed Mar 28 10:00:30 2018 -0400

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

himdel · 2018-05-18T11:36:14Z

Fine BZ: ~~https://bugzilla.redhat.com/show_bug.cgi?id=1579501~~

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

AuthenticationAPI - use base64encode which can deal with UTF8 passwords (cherry picked from commit 0ae3d01) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1562798

simaishi · 2018-05-18T13:01:37Z

Fine backport details:

$ git log -1
commit 577c5533bdc412df10b2730c283253f36ff658ae
Author: Allen Wight <allen.b.wight@gmail.com>
Date:   Wed Mar 28 10:00:30 2018 -0400

    Merge pull request #1411 from himdel/password-utf-bz1527316
    
    AuthenticationAPI - use base64encode which can deal with UTF8 passwords
    (cherry picked from commit 0ae3d013fe0c6211080adb28f9c8130ba3ed1bdc)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1562798

simaishi · 2018-05-18T13:42:58Z

@himdel Can you please take a look at Travis failure in Fine branch?

https://travis-ci.org/ManageIQ/manageiq-ui-service/jobs/380680697

himdel · 2018-05-18T14:28:04Z

Aah, looks like the failure is just linters .. doing a FINE PR to fix... => #1434

Remove angular-base64

83273df

suffers from the same problem as window.btoa - can't handle unicode strings https://bugzilla.redhat.com/show_bug.cgi?id=1527316

himdel added bug blocker dependencies gaprindashvili/yes labels Mar 26, 2018

himdel added 2 commits March 26, 2018 17:08

AuthenticationAPI - use utf8 aware base64encode instead of $base64.en…

bdbc83b

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

AllenBW previously approved these changes Mar 26, 2018

View reviewed changes

AuthenticationApi - update TextEncoderLite to a version which exports…

1384c4a

… the encoder and explicitly say window.TextEncoder for when it doesn't exist

himdel dismissed AllenBW’s stale review via 1384c4a March 27, 2018 14:10

AllenBW reviewed Mar 27, 2018

View reviewed changes

Use concise repo reference

eeb03c9

AllenBW approved these changes Mar 28, 2018

View reviewed changes

AllenBW merged commit 0ae3d01 into ManageIQ:master Mar 28, 2018

AllenBW assigned AllenBW and simaishi Mar 28, 2018

simaishi added gaprindashvili/backported and removed gaprindashvili/yes labels Apr 2, 2018

himdel deleted the password-utf-bz1527316 branch May 18, 2018 11:35

himdel added the fine/yes label May 18, 2018

simaishi added fine/backported and removed fine/yes labels May 18, 2018

himdel mentioned this pull request May 18, 2018

[FINE] AuthenticationApi factory - fix fine-specific linter issues #1434

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AuthenticationAPI - use base64encode which can deal with UTF8 passwords #1411

AuthenticationAPI - use base64encode which can deal with UTF8 passwords #1411

himdel commented Mar 26, 2018

himdel commented Mar 26, 2018

AllenBW left a comment

AllenBW commented Mar 26, 2018

himdel commented Mar 26, 2018 •

edited

himdel commented Mar 27, 2018 •

edited

miq-bot commented Mar 27, 2018

himdel commented Mar 27, 2018 •

edited

AllenBW Mar 27, 2018

AllenBW left a comment

AllenBW commented Mar 28, 2018

simaishi commented Apr 2, 2018

himdel commented May 18, 2018 •

edited

simaishi commented May 18, 2018

simaishi commented May 18, 2018

himdel commented May 18, 2018 •

edited

AuthenticationAPI - use base64encode which can deal with UTF8 passwords #1411

AuthenticationAPI - use base64encode which can deal with UTF8 passwords #1411

Conversation

himdel commented Mar 26, 2018

himdel commented Mar 26, 2018

AllenBW left a comment

Choose a reason for hiding this comment

AllenBW commented Mar 26, 2018

himdel commented Mar 26, 2018 • edited

himdel commented Mar 27, 2018 • edited

miq-bot commented Mar 27, 2018

himdel commented Mar 27, 2018 • edited

AllenBW Mar 27, 2018

Choose a reason for hiding this comment

AllenBW left a comment

Choose a reason for hiding this comment

AllenBW commented Mar 28, 2018

simaishi commented Apr 2, 2018

himdel commented May 18, 2018 • edited

simaishi commented May 18, 2018

simaishi commented May 18, 2018

himdel commented May 18, 2018 • edited

himdel commented Mar 26, 2018 •

edited

himdel commented Mar 27, 2018 •

edited

himdel commented Mar 27, 2018 •

edited

himdel commented May 18, 2018 •

edited

himdel commented May 18, 2018 •

edited