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

Don't check signup hook content-length #326

Closed
mikeslattery opened this issue Sep 1, 2022 · 2 comments
Closed

Don't check signup hook content-length #326

mikeslattery opened this issue Sep 1, 2022 · 2 comments
Labels

Comments

@mikeslattery
Copy link

mikeslattery commented Sep 1, 2022

- Do you want to request a feature or report a bug? Bug

- What is the current behavior?

The signup hook response is not parsed, if it doesn't consist of a Content-Type header value. In my hook I was trying to set app_metadata.roles but my value is ignored.

Here's the code and the blame.I think this was added to avoid json parse errors, but POST responses can often have no known length. I think there should still be a check, but on the body text itself, not the header value.

- If the current behavior is a bug, please provide the steps to reproduce.

I stripped out error handling and left out some details that any gotrue dev should know, but this code should work. I copy/pasted snippets from my project.

  1. Create and launch an empty netlify nuxt project with netlify-identity. You may need to register with netlify.
yarn create nuxt-app gotrue-test
cd gotrue-test
yarn add -g netlify-cli
yarn add netlify-identity-widget
yarn add -D dotenv jest
netlify dev --live
  1. Write a functions/identity-signup/identity-signup.js, that doesn't return a content-length:
exports.handler = function (event, context, callback) {
  const bodyJson = JSON.parse(event.body).user
  if (bodyJson.event === 'signup') {
    bodyJson.app_metadata.roles = ['regular']
  }
  const bodyText = JSON.stringify(bodyJson)
  callback(null, {
    statusCode: 200,
    headers: {
      // WORKAROUND: Uncomment next line as a fix
      // 'Content-Length': bodyText.length,
    },
    body: bodyText,
  })
}
  1. Write .env for gotrue and jest to auto-confirm, auto-signup
# .env file
GOTRUE_DISABLE_SIGNUP='false'
GOTRUE_MAILER_AUTOCONFIRM='true'
GOTRUE_SITE_URL='http://localhost:8081'
GOTRUE_WEBHOOK_URL='http://localhost:8888/.netlify/functions/identity-signup'
  1. Run gotrue
source .env
export GOTRUE_DISABLE_SIGNUP GOTRUE_MAILER_AUTOCONFIRM GOTRUE_WEBHOOK_URL
gotrue
  1. Write and run this jest code:
/**
 * @jest-environment jsdom
 */

describe('Tests', () => {
  test('Missing Content Type', () => {
    const env = require('dotenv').config().parsed
    const netlifyIdentity = require('netlify-identity-widget')
    netlifyIdentity.init({
        APIUrl: env.GOTRUE_SITE_URL,
    })
    await netlifyIdentity.store.signup('Netlify Tester', 'tester@netlify.com', 'password')
    await netlifyIdentity.store.login('tester@netlify.com', 'password')
    
    // This FAILS
    expect(netlifyIdentity.store?.user?.app_metadata?.roles).toEqual(['regular'])
  }
}

- What is the expected behavior?

app_metadata should be modified. See test code above. If you uncomment my workaround, it works correctly.

- Please mention your Go version, and operating system version.

1.15 as comes with golang:1.15-alpine docker image. I use this to build gotrue. This is the same as in go.mod.

@github-actions
Copy link

github-actions bot commented Sep 3, 2023

This issue has been automatically marked as stale because it has not had activity in 1 year. It will be closed in 7 days if no further activity occurs. Thanks!

@github-actions github-actions bot added the stale label Sep 3, 2023
@mikeslattery
Copy link
Author

This appears to have been fixed by pull #342

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant