PSA: Avoid passing object reference to constructor to avoid state leakage
tobobo opened this issue · 1 comments
I just ran in to this while testing an app with an auth code grant flow before release and thought it might be helpful for others to avoid the hard-to-debug issue I ran in to.
If your app has your client ID and secret stored in an object, make sure to pass a copy of that object to the constructor to avoid state leakage.
Say you have a route that performs an authorization code grant
// spotifyCredentials.js
export const credentials = {
clientId: process.env.SPOTIFY_CLIENT_ID,
clientSecret: process.env.SPOTIFY_CLIENT_SECRET,
};
// server.js
import { credentials } from './spotifyCredentials';
import express from 'express';
const app = express();
app.use((req, res, next) => {
// middlware to authenticate user and add req.user with spotify credentials
});
app.post('/spotify_auth_code_grant', (req, res) => {
const spotifyWebApi = new SpotifyWebApi(credentials); // DANGER!! spotifyWebApi can now modify const credentials
const authResponse = await spotifyWebApi.authorizationCodeGrant(req.body.code);
const { access_token: accessToken, refresh_token: refreshToken } = authResponse.body;
// these two lines now modify the original const credentials
spotifyWebApi.setAccessToken(accessToken);
spotifyWebApi.setRefreshToken(refreshToken);
const userResponse = await spotifyWebApi.getMe();
// ...save spotify account data to User model or something
res.status(201).end();
});
app.get('/spotify_currently_playing', (req, res) => {
const { spotifyAccessToken: accessToken, spotifyRefreshToken } = req.user;
const spotifyWebApi = new SpotifyWebApi({
accessToken,
refreshToken,
...credentials // DANGER!! Credentials can have accessToken and refreshToken from last request to /spotify_auth_code_grant
});
const nowPlayingResponse = await spotifyWebApi.getMyCurrentlyPlayingTrack();
res.json(nowPlayingResponse.body);
});
Any request to /spotify_currently_playing
will have the tokens for its SpotifyWebApi
instance overwritten by the tokens from the last user to call /spotify_auth_code_grant
.
This could be fixed by putting the ...credentials
spread from /spotify_currently_playing
before accessToken
and refreshToken
, but the best fix is to make a shallow copy of credentials
when making the SpotifyWebApi
request in /spotify_auth_code_grant
:
const spotifyWebApi = new SpotifyWebApi({ ...credentials });
Hope this saves someone some time!
Good catch, spent 20 minutes debugging that issue. 🙄