Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adding concur provider #34

Closed
dominiksteiner opened this issue Sep 25, 2015 · 11 comments
Closed

adding concur provider #34

dominiksteiner opened this issue Sep 25, 2015 · 11 comments

Comments

@dominiksteiner
Copy link

A colleague created a fork in order to add concur as a provider to grant.

master...szafarcin:master

Wondering if there is a best practice on how to add new providers to grant? Can you assess the above fork on how practical it would be to merge it back into grant?

Thanks

@simov
Copy link
Owner

simov commented Sep 25, 2015

Hi, @dominiksteiner, thanks for reaching out. I noticed that fork, and I'll take a look at it these days 👍

Generally adding a new provider is just a matter of adding a configuration in the config/oauth.json. The amount of other changes needed depends on how much exactly the provider in question deviates from the official OAuth specification.


On a side note: having configuration like gdrive and gmail in config/oauth.json is not needed. You can define static overrides in your configuration:

"google": {
  "gdrive": {"scope": []},
  "gmail": {"scope": []}
}

Then navigate to /connect/google/gdrive or /connect/google/gmail.

@simov
Copy link
Owner

simov commented Sep 25, 2015

There you go 2ecc996 @szafarcin @dominiksteiner let me know what do you think.

The next release will be published the next week, so in the meantime you can pull master and test.

The output format is as follows:

{
  "access_token": "...",
  "refresh_token": "...",
  "raw": "raw xml string"
}

If you need anything else than the access_token and the refresh_token you can parse the raw key on your own.

@dominiksteiner
Copy link
Author

wow, incredible, looks good, will test it, thanks for this quick integration.

@simov
Copy link
Owner

simov commented Sep 25, 2015

Also what's currently missing from the docs is that the following is identical:

// Express
var Grant = require('grant-express')
var Grant = require('grant').express()
// Koa
var Grant = require('grant-koa')
var Grant = require('grant').koa()
// Hapi
var Grant = require('grant-hapi')
var Grant = require('grant').hapi()

For Koa you need two additional dependencies: thunkify and koa-route

So you can require Grant directly in your app, without the need of meta module.

@dominiksteiner
Copy link
Author

Thanks, that's useful information too.

@szafarcin
Copy link

@simov, just tested concur change seems to be returning access/refresh tokens fine.

However there is a slight further requirement based on the same fork. We are passing custom variables to corresponding callback if they are present in initial request. If you take a look at utils.js, we are binding variables (id,pwd,xsp,rurl etc) to provider and retrieving them in the callback for further use.
What do you suggest a best way to utilize such feature in grant?

Thanks

@simov
Copy link
Owner

simov commented Sep 28, 2015

Hi, @szafarcin, I'm not seeing such parameters in the official API docs.

That seems to be part of your own application logic. If I'm understand you correctly you want to store some user data and use it in the final callback? If that's the case the easiest way would be to store it in the user's session on connect and then access it in the final callback.

@szafarcin
Copy link

Hi @simov ,

Yes correct, I was referring to variables mapped in https://github.com/szafarcin/grant/blob/master/lib/utils.js

Correct me if I am wrong regarding storing in session wouldn't that be part of grant itself as "connect" is called inside, and external app which uses grant just has the callback hook?. So what do you suggest for hooking data without changing in grant master. Thanks

@simov
Copy link
Owner

simov commented Sep 28, 2015

I think we're on the same page. The first thing that comes to my mind is this:

// navigate to /connect_concur instead of /connect/concur
app.use('/connect_concur', function (req, res) {
  req.session.user = {
    // store the current user's data
  }
  res.redirect('/connect/concur')
})

app.use('/final_callback', function (req, res) {
  // depending on your configuration
  // the OAuth response data is either in:
  req.query
  // or
  req.session.grant
  // the user's data that you stored previously in the connect route is in:
  req.session.user
})

Storing user data in grant.config is actually a bad practice. If you have two users connecting at the same time, the user data there will be incorrect. That's why it's better to store the user's data in its session.

@szafarcin
Copy link

@simov thanks a lot for the clarification, and valuable input.

@simov
Copy link
Owner

simov commented Sep 30, 2015

Version 3.5.1 is published.

@simov simov closed this as completed Sep 30, 2015
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

No branches or pull requests

3 participants