Skip to content

Conversation

@stefanpante
Copy link
Member

No description provided.

"license": "ISC",
"dependencies": {
"@digipolis/auth": "^1.1.0",
"@digipolis/auth": "git+https://github.com/digipolisantwerp/auth_module_nodejs.git",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waarom is dit niet de gepubliceerde versie?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

omdat deze versie nog niet gepublished is.

```js
app.use(require('@digipolis/auth')(app, configuration));
```
You should use `express-session` in your application to enable session storage.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Misschien handig om achter express-session een linkje te leggen naar de juiste module:
https://www.npmjs.com/package/express-session

}

function logout(req, res) {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whiteline :-o

}

async function refreshToken(req, res, next) {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whiteline

];

function validateConfig(config) {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whiteline

});

res.on('end', () => {
// assert(res.redirect.calledWith('/'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oeps?

test/sso.js Outdated


describe('test sso middleware', function onDescribe() {
// nockGetSession({ssoKey: 'fakessokey', payload: emptySessions})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mag weg.

- PORT=2000
- MONGO_CONNECTIONSTRING=mongodb://mongo:27017/basic-mongo-auth
- CLIENT_SECRET=
- CLIENT_ID
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Misschien handig om in te vullen met dummy code?

@Jan-Bart
Copy link
Member

Ik zou ook node v6 uit de test-builds halen en expliciet vermelden in de README dat we dit niet meer ondersteunen.

Copy link
Collaborator

@frankdsm frankdsm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only es module imports pls

}

const newToken = await service.refresh(token);
req.session = Object.assign(req.session, { [tokenKey]: newToken });
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rest operator can be used here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maakt rest operator er geen nieuw object van, ik kopieer hier de property op de req.session
Misschien is de assignment dan niet echt nodig.

export function getHost(req) {
return `${req.protocol}://${req.get('host')}`
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check for new lines with eslint

const user = require('./mocks/user.json');

describe('test #isLoggedin', function onDescribe() {
describe('GET /isLoggedin', function onDescribe() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lowercase urls pls

mongoConnectionString,
port = 2000
} = require('./config');
} = require('./old-config');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does te example use an old config ?

package.json Outdated
Comment on lines 29 to 34
"@babel/core": "^7.8.4",
"@babel/preset-env": "^7.8.4",
"@babel/register": "^7.8.3",
"@istanbuljs/nyc-config-babel": "^3.0.0",
"@rollup/plugin-node-resolve": "^7.1.1",
"babel-plugin-istanbul": "^6.0.0",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to add babel to this?
I see a target to node 8 however i would tend to target the latest lts versions for new modules.
node 8 is passed its lifetime anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants