Adding the ability to map attributes in request.session to request.auth.credentials #7
Conversation
api.md
Outdated
| @@ -84,5 +84,6 @@ with the scheme name 'cas'. | |||
| | includeHeaders | <code>array</code> | <code>['cookie']</code> | The headers to include in redirections. This list <em>must</em> include the header your session manager uses for tracking session identifiers. | | |||
| | strictSSL | <code>boolean</code> | <code>true</code> | Determines if the client will require valid remote SSL certificates or not. | | |||
| | saveRawCAS | <code>boolean</code> | <code>false</code> | If true the CAS result will be saved into session.rawCas | | |||
| | sessionCredentialsMappings | <code>Array</code> | | An array of objects where the values of the attribute of <code>request.session</code> listed in<code>object.sessionAttribute</code> will be mapped to the attribute of <code>request.auth.credentials</code> listed in <code>object.credentialsAttribute</code>. For example, if <code>sessionCredentialsMappings</code> contains <code>['foo.bar', 'baz']</code> then <code>request.auth.credentials.baz</code> will contain the same data as <code>request.session.foo.bar</code>. <strong>NOTE</strong>: dot notation in the array elements is supported. | | |||
There was a problem hiding this comment.
This is really confusing, and the example doesn't match the updated code.
There was a problem hiding this comment.
Oops, forgot to rewrite some parts to account for using an object instead of an array. I don't know how else to simplify the explanation though :/
plugin.js
Outdated
| @@ -54,6 +58,11 @@ const optsSchema = Joi.object().keys({ | |||
| includeHeaders: Joi.array().items(Joi.string()).default(['cookie']), | |||
| strictSSL: Joi.boolean().default(true), | |||
| saveRawCAS: Joi.boolean().default(false), | |||
| // sessionCredentialsMappings: Joi.number().required(), | |||
plugin.js
Outdated
| @@ -159,6 +168,12 @@ function casPlugin (server, options) { | |||
| username: session.username, | |||
| attributes: session.attributes | |||
| } | |||
| if (_options.value.sessionCredentialsMappings) { | |||
| const dotProp = require('dot-prop') | |||
There was a problem hiding this comment.
Imports lead the file (stdlib first, third party second, and local third) unless they need to be scoped to a function. This one doesn't.
There was a problem hiding this comment.
Done. Just trying to not clutter the top as it may not be used.
plugin.js
Outdated
| @@ -159,6 +168,12 @@ function casPlugin (server, options) { | |||
| username: session.username, | |||
| attributes: session.attributes | |||
| } | |||
| if (_options.value.sessionCredentialsMappings) { | |||
| const dotProp = require('dot-prop') | |||
| for (let mapping of _options.value.sessionCredentialsMappings) { | |||
There was a problem hiding this comment.
Don't use for..of. Either use a full for(;;) loop or a .forEach (in that preference order).
plugin.js
Outdated
| if (_options.value.sessionCredentialsMappings) { | ||
| const dotProp = require('dot-prop') | ||
| for (let mapping of _options.value.sessionCredentialsMappings) { | ||
| dotProp.set(credentials, mapping.credentialsAttribute, dotProp.get(session, mapping.sessionAttribute)) |
There was a problem hiding this comment.
There's a lot of work going on here for something that is really simple:
https://github.com/jsumners/nixconfig/blob/master/lib/getPathValue.js
https://github.com/jsumners/nixconfig/blob/37987334d80ddff24a2b815a0747ffd4b7093f4d/lib/nixconfig.js#L91-L103
Do we really need the extra two dependencies (1 direct & 1 indirect) for this?
plugin.js
Outdated
| @@ -159,6 +168,12 @@ function casPlugin (server, options) { | |||
| username: session.username, | |||
| attributes: session.attributes | |||
| } | |||
| if (_options.value.sessionCredentialsMappings) { | |||
| const dotProp = require('dot-prop') | |||
| for (let mapping of _options.value.sessionCredentialsMappings) { | |||
There was a problem hiding this comment.
No description provided.