[keycloak-dev] Javascript Adapter Feedback

[Guillaume Vincent]

Hello,

this mail is an email to give my feedback on Keycloak Javascript Adapter.
The feedback is not good. If you are not open to criticism, do not read
this email.

TLDR: The Javascript code is a 1500+ line of code, untested, with 10 levels
of indentations, no abstraction, difficult to read, difficult to maintain,
include in an web application and not following semver.

# loc
maintain and fix 1500 line of code is not easy at all. The javascript
adapter should be split in small javascript functions/modules/class and
respect some basic clean code. SRP, no duplication, etc.

# tests
No single test on the javascript adapter? really. We should introduce some
basic tests and API test to avoid breaking the interface. Basically
decrypting a JWT, parsing the url, initialisation could be tested easily.

# semver
In javascript world, the common convention regarding the version number is
semver[1]. So when the API change, the major version should be incremented.
Not the minor one. We could avoid like that fixing the version number in
package.json [2] to avoid those kind of bug [3]

# integration in existing app
nobody today create real professional frontend application using the plain
javascript file. The frontend world is compose of small piece assemble
together. We use bundler like webpack, browserify, rollup etc to create our
application. Basically we don't do this:

```
<script src="keycloak.js"></script>
<script>
  var keycloak = Keycloak();
  keycloak.init()
</script>
```

we do this

```
const Keycloack = require("keycloack");
var keycloak = Keycloak();
keycloak.init();

```

yes like in node


# modules
the keycloak library should use modules or be decomposed into modules.
Decoupled in small piece and use rollup or webpack to bundle the
application. It will help for example bundler to do tree shacking for
example and reduce the bundle size of the library.

So instead of try to reinvent the wheel and introduce some insecure
code[4]. We should use javascript module, well tested like this one[5] and
do something like

const uuidv4 = require('uuid/v4');
function createUUID() {
  return uuidv4();
}

# hashtag
redirectURL doesn't work if it contains a hashtag. Popular framework
AngularJS, VueJS use hastag technique for routing.

# cookies, iframe
This make our work as developer trully hard. Basically the simplest
javascript adpater, should be

Instantiate a keycloak singleton
load existing jwt in localstorage if present
load url params to see if jwt in the url
if jwt is present, save in local storage
update the singleton

# cordova adapter
I don't look at this code, but I'm just asking myself, why a javascript
adapter need a special code for cordova application?

# keycloak promise Oo
https://www.keycloak.org/docs/latest/upgrading/index.html#javascipt-adapter-promise
A promise is standardized in javascript, so you should avoid using this
term in the code if you don't return a promise. callback is a better term.
A promise should return a unfulfilled method with then and catch methods
attach to it.




In French we have a proverb: "Criticism is easy, art is difficult". So I
will definitely create a smaller javascript adapter that does not suffer
from the problems listed above. And I would come and show the code here.

Thanks for reading me


[1] https://semver.org/
[2]
https://github.com/redhat-cip/dci-ui/commit/e4d0208128afa05c33e72abe606cdcd93594c93e
[3] https://issues.jboss.org/browse/KEYCLOAK-8839
[4]
https://github.com/keycloak/keycloak/blob/master/adapters/oidc/js/src/main/resources/keycloak.js#L882-L893
[5] https://github.com/kelektiv/node-uuid

-- 

Guillaume  Vincent

Senior FrontEnd Engineer

gvincent at redhat.com
<https://red.ht/sig>
TRIED. TESTED. TRUSTED. <https://redhat.com/trusted>