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

feat(authentication-oauth): callback for proxying raw request params into authentication service #1869

Conversation

mikekolganov
Copy link

@mikekolganov mikekolganov commented Mar 15, 2020

Problem: I can't get user's IP address on registration and login via OAuth.

This PR was created after this issue: #1833

Also I have related PR to feathersjs/docs#1447

Case: I have app with Facebook OAuth login flow (JWT). I need to save users IP addresses when they're registered or logged in and expected to use hooks for authentication service. Middleware for throwing raw request data to Feathers (IP address in this case) is added to middleware/index.js, assigning new keys to req.feathers object. I can see this data in context.params of all hooks except authentication service hooks (after: create()). I noticed that params are limited here and not carrying any of custom params from req.feathers object in authentication-oauth/src/express.ts

Example of use

middleware:

  app.use((req, res, next) => {
    if (! req.feathers) {
      req.feathers = {};
    }

    req.feathers.ip = req.connection.remoteAddress;
    req.feathers.userAgent = req.headers ['user-agent'];
    next();
  });

authentication config:

  app.configure(expressOauth({
    serviceParamsCallback: req => {
      const { ip, userAgent } = req.feathers;

      return { ip, userAgent };
    },
  }));

@mikekolganov mikekolganov changed the title authentication-oauth: callback for proxying raw request params into authentication service feat(authentication-oauth): callback for proxying raw request params into authentication service Mar 15, 2020
@daffl
Copy link
Member

daffl commented Mar 19, 2020

Thank you for the pull request. I think supporting this makes sense but couldn't we still just use req.feathers from the middleware by changing

to

const params = {
  ...req.feathers,
  authStrategies: [ name ],
  authentication: accessToken ? {
    strategy: linkStrategy,
    accessToken
  } : null,
  query,
  redirect
};

@daffl
Copy link
Member

daffl commented Mar 23, 2020

I added a PR that implements my suggestion in #1886. I think it solves the same issue without adding an additional option.

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.

None yet

2 participants