Adding secondary session sharing #11

enoodle · 2016-12-15T17:38:13Z

Adding two endpoints "/auth/sso-setup" and "/auth/sso-login" to allow giving a 3rd the ability to authenticate without actually sharing the token with it.

@jimmidyson @sosiouxme @simon3z

jcantrill · 2016-12-15T18:27:25Z

lib/authHandlers.js

+    console.log("EREZ handleSSOLogin: req.query = ", req.query);
+    if (req.query.user_token) {
+      console.log("EREZ handleSSOLogin: req has user token!!");
+      req.query.access_token = secondarySessionTokens[req.query.user_token];


What are the implications of not having a session token in the cache?

I think we can simply remove the token from the cache once the new session takes it and insert it into it's session.

What are the implications of not having a session token in the cache?

if ManageIQ did not insert a new session token to the cache ~immediately before the user starts a new session, it means the user is not authorized. This is why it is safe to remove the token from the cache once it's consumed by the user.

What are the implications of not having a session token in the cache?
Do you mean what happens if req.query.user_token is not in secondarySessionTokens?
Then req.query.access_token will be set a undefined and the authentication using it will fail and the user will be redirected to authenticate with Openshift.

jcantrill · 2016-12-15T18:28:18Z

lib/authHandlers.js

@@ -8,6 +8,8 @@ var sessions       = require('client-sessions'),
    config         = require('./config'),
    parseDuration  = require('parse-duration');

+var secondarySessionTokens = {}


Shouldn't we be concerned with the cache growing without bounds? Do we need to purge periodically?

We can also keep just one (Two, Three, N ?) and pop it once it is used. How does that sound?

enoodle · 2016-12-18T14:47:53Z

@jcantrill I have changed to use an LRU cache to store the user token. I added a dependency for "lru_map" for that. Please tell me if this is acceptable.

jcantrill · 2016-12-19T14:42:48Z

lib/authHandlers.js

@@ -7,6 +7,10 @@ var sessions       = require('client-sessions'),
    bodyParser     = require('body-parser'),
    config         = require('./config'),
    parseDuration  = require('parse-duration');
+    LRUMap         = require('lru_map');
+
+var secondarySessionTokens = new LRUMap.LRUMap(10);


I would allow this to be initialized via an env var that defaults to 10. Concerns here of what this would mean when used at scale.

👍 will do

jcantrill · 2016-12-19T14:44:08Z

package.json

@@ -26,6 +26,7 @@
    "passport-oauth2": "^1.1.2",
    "request": "^2.60.0",
    "url-join": "0.0.1",
-    "yargs": "^3.18.0"
+    "yargs": "^3.18.0",
+    "lru_map": "^0.3.2"


@richm This will have impact on packaging the auth proxy for openshift.

enoodle · 2017-01-05T16:41:41Z

I joined all the commits here and updated handleSSOSetup to return 200 to the client.
In addition I also added a commit to bump the base docker image of node, this was needed for the LRU cache
I guess it won't be accepted as is but this is something we need to discuss to see how we solve. I could also try to implement LRU or similar solution, but I prefer to see if bumping the base image is acceptable first.
@jcantrill PTAL

richm · 2017-01-05T17:33:39Z

Dockerfile

@@ -1,4 +1,4 @@
-FROM node:0.10.36
+FROM node:7.3


Can you use node:4.7? This will make downstream integration much easier.

No.. It throws this error:

/usr/local/lib/node_modules/openshift-auth-proxy/node_modules/lru_map/lru.js:78 let entry, limit = this.limit || Number.MAX_VALUE; ^^^ SyntaxError: Block-scoped declarations (let, const, function, class) not yet supported outside strict mode at exports.runInThisContext (vm.js:53:16) at Module._compile (module.js:373:25) at Object.Module._extensions..js (module.js:416:10) at Module.load (module.js:343:32) at Function.Module._load (module.js:300:12) at Module.require (module.js:353:17) at require (internal/module.js:12:17) at Object.<anonymous> (/usr/local/lib/node_modules/openshift-auth-proxy/lib/authHandlers.js:10:22) at Module._compile (module.js:409:26) at Object.Module._extensions..js (module.js:416:10)

I will see how to change this module if it is not possible to accommodate this.

So I changed lru_map to lru-cache and all seem to work now without touching the base node image. I guess this is the best solution.

jcantrill · 2017-01-05T18:42:08Z

LGTM. I defer to @richm and any objections he may have. I'm mostly concerned, if anything, with the production implications of the node change and the added libraries.

Allowing clients to stash the authentication token in the proxy and transfer the session to another user/client with a different token. Users tokens are saved in an LRU cache whose size is configurable.

enoodle · 2017-01-17T11:39:40Z

ping @jcantrill @richm I changed to use a different LRU that doesn't require any image changes. PTAL

jcantrill · 2017-01-17T16:52:52Z

LGTM

enoodle changed the title ~~Adding secondary session sharing~~ [WIP] Adding secondary session sharing Dec 15, 2016

jcantrill reviewed Dec 15, 2016

View reviewed changes

enoodle force-pushed the adding_secondary_session_sharing branch from 0b22254 to b45f901 Compare December 18, 2016 15:21

enoodle changed the title ~~[WIP] Adding secondary session sharing~~ Adding secondary session sharing Dec 18, 2016

enoodle mentioned this pull request Dec 18, 2016

[WIP] container provider: adding a link to hosted kibana ManageIQ/manageiq#10648

Closed

jcantrill reviewed Dec 19, 2016

View reviewed changes

enoodle force-pushed the adding_secondary_session_sharing branch from b45f901 to 5219d60 Compare December 20, 2016 15:11

enoodle mentioned this pull request Dec 26, 2016

Adding External Logging links to container providers and nodes ManageIQ/manageiq-ui-classic#36

Closed

enoodle force-pushed the adding_secondary_session_sharing branch from 5219d60 to 26fe567 Compare January 5, 2017 16:36

richm reviewed Jan 5, 2017

View reviewed changes

adding /auth/sso-{setup,login} endpoints

e8d20cf

Allowing clients to stash the authentication token in the proxy and transfer the session to another user/client with a different token. Users tokens are saved in an LRU cache whose size is configurable.

enoodle force-pushed the adding_secondary_session_sharing branch from 26fe567 to e8d20cf Compare January 8, 2017 11:43

jcantrill merged commit c9e67a3 into fabric8io:master Jan 24, 2017

enoodle mentioned this pull request Mar 2, 2017

Adding SSO to External Logging link ManageIQ/manageiq-ui-classic#550

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding secondary session sharing #11

Adding secondary session sharing #11

enoodle commented Dec 15, 2016 •

edited

Loading

jcantrill Dec 15, 2016

yaacov Dec 15, 2016

yaacov Dec 15, 2016

enoodle Dec 18, 2016

jcantrill Dec 15, 2016

enoodle Dec 18, 2016

enoodle commented Dec 18, 2016

jcantrill Dec 19, 2016

enoodle Dec 19, 2016

enoodle Dec 21, 2016

jcantrill Dec 19, 2016

enoodle commented Jan 5, 2017

richm Jan 5, 2017

enoodle Jan 8, 2017

enoodle Jan 8, 2017

jcantrill commented Jan 5, 2017

enoodle commented Jan 17, 2017

jcantrill commented Jan 17, 2017

Adding secondary session sharing #11

Adding secondary session sharing #11

Conversation

enoodle commented Dec 15, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enoodle commented Dec 18, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enoodle commented Jan 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcantrill commented Jan 5, 2017

enoodle commented Jan 17, 2017

jcantrill commented Jan 17, 2017

enoodle commented Dec 15, 2016 •

edited

Loading