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

add node example #1

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

add node example #1

wants to merge 1 commit into from

Conversation

stevekaliski-stripe
Copy link
Collaborator

No description provided.

Copy link
Collaborator

@vbc-stripe vbc-stripe left a comment

Choose a reason for hiding this comment

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

This looks like it's on the right track, though a few things are undefined for me. Could it be that I'm using the wrong version of node? npm install worked fine.

$ node -v
v18.14.0


// Listens to all network interfaces. This allows any device on your
// network to connect.
app.listen(port, '0.0.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.

port is undefined here i think?

const cardholderDB = {
[`${env.USERNAME}-${env.PASSWORD}`]: env.CARDHOLDER_ID
};
return cardholderDB[req.headers.authorization.split(' ')[1]];
Copy link
Collaborator

@vbc-stripe vbc-stripe Nov 29, 2023

Choose a reason for hiding this comment

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

authorization is undefined here. This is because this backend doesn't issue a challenge along with the 401 response.

Please see this comment for details.

});

function authenticate(req) {
return req.session.authenticated && req.session.cardholderId;
Copy link
Collaborator

Choose a reason for hiding this comment

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

session is undefined here

// Replace if using a different env file or config
const env = require('dotenv').config({ path: './.env' });

const stripe = require('stripe')(process.env.STRIPE_SECRET_KEY, {
Copy link
Collaborator

Choose a reason for hiding this comment

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

we call this STRIPE_LIVE_SECRET_KEY in the ruby .env: https://github.com/vbc-stripe/push-provisioning-samples/blob/main/server/ruby/.env.example#L2

i'm ok with picking either one and sticking with it for both backend impls.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i think we'll stick with the STRIPE_SECRET_KEY for consistency with other sample code https://github.com/stripe-samples/template/blob/main/.env.example

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok, changed everything from STRIPE_LIVE_SECRET_KEY to STRIPE_SECRET_KEY: 30a2cbf

if (authenticate(req)) {
try {
const key = await stripe.ephemeralKeys.create(
{ issuing_card: req.body.card_id },
Copy link
Collaborator

Choose a reason for hiding this comment

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

api_version and card_id are application/x-www-form-urlencoded, perhaps that's why card_id and api_version are undefined on req.body for me.

After some googling, i found a way to parse these params from the request body:

      const body = await getBody(req);
      const params = new URLSearchParams(body);
      const key = await stripe.ephemeralKeys.create(
        { issuing_card: params.get('card_id') },
        { stripe_version: params.get('api_version') }
      );

and here's getBody:

function getBody(request) {
  return new Promise((resolve) => {
    const bodyParts = [];
    let body;
    request.on('data', (chunk) => {
      bodyParts.push(chunk);
    }).on('end', () => {
      body = Buffer.concat(bodyParts).toString();
      resolve(body)
    });
  });
}

try {
const key = await stripe.ephemeralKeys.create(
{ issuing_card: req.body.card_id },
{ stripe_version: req.body.api_version }
Copy link
Collaborator

@vbc-stripe vbc-stripe Nov 29, 2023

Choose a reason for hiding this comment

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

got this deprecation warning:

Stripe: 'stripe_version' is deprecated; use 'apiVersion' instead.

still works for now, though

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

res.status(404).send(`Error listing cards: ${error.message}`);
}
} else {
res.status(401).send('Not authorized');
Copy link
Collaborator

@vbc-stripe vbc-stripe Nov 30, 2023

Choose a reason for hiding this comment

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

In our ruby implementation of http basic auth, when the client sends a request to the server without an authorization header, it too responds with a 401 status and also a WWW-Authenticate header with a value of 'Basic realm="Restricted Area"' which is the challenge[1]: https://github.com/vbc-stripe/push-provisioning-samples/blob/main/server/ruby/server.rb#L94

When the client receives this 401 response with the challenge in the WWW-Authenticate header, it reissues the request, this time with the authorization request header set to "$username:$password"

[1]:

Upon receipt of a request for a URI within the protection space that
lacks credentials, the server can reply with a challenge using the
401 (Unauthorized) status code ([RFC7235], Section 3.1) and the
WWW-Authenticate header field ([RFC7235], Section 4.1).

-- https://datatracker.ietf.org/doc/html/rfc7617#section-2

@vbc-stripe
Copy link
Collaborator

@stevekaliski-stripe heads i transferred this repo to the stripe samples organization at https://github.com/stripe-samples/push-provisioning-samples

i don't think that should affect this PR until you push any subsequent commits, might need to update your remote.

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.

2 participants