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

Implicit scope is not used and isn't sufficient #21

Open
jnv opened this issue Nov 21, 2022 · 2 comments
Open

Implicit scope is not used and isn't sufficient #21

jnv opened this issue Nov 21, 2022 · 2 comments
Labels
bug Something isn't working

Comments

@jnv
Copy link
Collaborator

jnv commented Nov 21, 2022

In #11 a default scope of users.read was introduced.

However, when creating a minimum viable server for this strategy I noticed two issues with this implicit behavior:

  • Scope users.read is insufficient for accessing user's profile, the endpoint requires both users.read and tweet.read (probably to access the pinned tweet, regardless whether it is requested or not)
  • Probably more significant, the default scope isn't picked up during authenticate call. Debugging the issue, I found that the underlying OAuth2 Strategy reads the default scope from _scope if not passed in during the authenticate call:

https://github.com/jaredhanson/passport-oauth2/blob/5fcd4c5238a3785794c8c8d0b4a1ab94f1f67bc1/lib/strategy.js#L231

In retrospect, I think this functionality couldn't work. Which is weird, because the test seem to asses this functionality.

Here is the code I am using:

const express = require('express');
const passport = require('passport');
const { Strategy } = require('@superfaceai/passport-twitter-oauth2');
const session = require('express-session');
require('dotenv').config();

// <1> Serialization and deserialization
passport.serializeUser(function (user, done) {
  done(null, user);
});
passport.deserializeUser(function (obj, done) {
  done(null, obj);
});

// Use the Twitter OAuth2 strategy within Passport.
passport.use(
  // <2> Strategy initialization
  new Strategy(
    {
      clientID: process.env.TWITTER_CLIENT_ID,
      clientSecret: process.env.TWITTER_CLIENT_SECRET,
      clientType: 'confidential',
      callbackURL: `${process.env.BASE_URL}/auth/twitter/callback`,
      skipUserProfile: false,
    },
    // <3> Success callback
    (accessToken, refreshToken, profile, done) => {
      console.log('Success!', { accessToken, refreshToken });
      return done(null, profile);
    }
  )
);

const app = express();

app.use(passport.initialize());
// <4> Session initialization
app.use(
  session({ secret: 'keyboard cat', resave: false, saveUninitialized: true })
);

// <5> Start authentication flow
app.get(
  '/auth/twitter',
  passport.authenticate('twitter', {
    // scope: ['tweet.read', 'users.read', 'offline.access'],
  })
);

// <6> Callback handler
app.get(
  '/auth/twitter/callback',
  passport.authenticate('twitter'),
  function (req, res) {
    const userData = JSON.stringify(req.user, undefined, 2);
    res.end(
      `<h1>Authentication succeeded</h1> User data: <pre>${userData}</pre>`
    );
  }
);

app.listen(3000, () => {
  console.log(`Listening on ${process.env.BASE_URL}`);
});

When running this and visiting http://localhost:3000/auth/twitter, I get redirected to Twitter without the scope URL parameter.

@jnv jnv added the bug Something isn't working label Nov 21, 2022
@jnv
Copy link
Collaborator Author

jnv commented Nov 21, 2022

I see two options here: fix the implicit behavior, or remove it in favor of documenting it.

@janhalama
Copy link
Collaborator

Thanks, I will add tests which will reproduce the issue and fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants