Skip to content

Webrtc 170 #12

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Webrtc 170 #12

wants to merge 4 commits into from

Conversation

asinghal-bandwidth
Copy link
Contributor

  • WEBRTC-170: Auto refresh token mechanism added

REACT_APP_ACCOUNT_DISPLAY_NAME=xxxxxxxxx
REACT_APP_ACCOUNT_PASSWORD=xxxxxxxxx
REACT_APP_AUTH_URL=https://authtoken.url

Choose a reason for hiding this comment

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

this will always be the id.bandwidth URL correct? Should we just set it for the user since it is not a secret value?

Choose a reason for hiding this comment

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

My only qualm with this is that we encourage customers to fetch auth tokens from the frontend, and storing the BW Username and pass there is not safe

it is a sample app at the end of the day, but the tokens shouldn't be generated client side, a backend should handle that so that the user/pass is nowhere in the client

// Your API details
const AUTH_URL = process.env.REACT_APP_AUTH_URL;
const header = process.env.REACT_APP_AUTH_CREDENTIALS;
const BASIC_AUTH_CREDENTIALS = btoa(header); // Base64 encoding for Basic Auth

Choose a reason for hiding this comment

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

instead of having repeated variables (username, password, and authCredentials which is just username:password) - can we simplify the env vars and construct the header from the provided user/pass env vars?

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.

3 participants