Username password auth #76
Conversation
| Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
| Have signed the CLA :) |
| CLAs look good, thanks! |
| */ | ||
| exports.auth = functions.https.onRequest((req, res) => { | ||
| try { | ||
| cors(req, res, () => { |
There was a problem hiding this comment.
Indent here is 4 space. Switch to two like the rest of the file (and the other samples) :)
There was a problem hiding this comment.
The rest of the file also has a bunch of 4 space indents :) please fix
| try { | ||
| cors(req, res, () => { | ||
| // Handle CORS preflight request | ||
| if (req.method === 'OPTIONS') { |
There was a problem hiding this comment.
I think that OPTIONS requests are already taken care of by the cors middleware. Probably you can remove this check.
| } | ||
| // For the purpose of this example use httpbin (https://httpbin.org) for the basic authentication request. | ||
| // (Only a password of `Testing123` will succeed) | ||
| let url = "https://httpbin.org/basic-auth/" + username + "/Testing123"; |
There was a problem hiding this comment.
Make sure you use single quotes everywhere for Strings and ctually here you could use String interpolation:
const url = `https://httpbin.org/basic-auth/${username}/Testing123`;| /** | ||
| * Authenticate the provided credentials returning a Firebase custom auth token. | ||
| * If authentication fails return a 401 response. | ||
| * If an error occurs log the details and return a 500 response. |
There was a problem hiding this comment.
Could you add i the comment here that the request expects a username and password values to be passed in the body of the request?
| } | ||
| // For the purpose of this example use httpbin (https://httpbin.org) for the basic authentication request. | ||
| // (Only a password of `Testing123` will succeed) | ||
| let url = "https://httpbin.org/basic-auth/" + username + "/Testing123"; |
There was a problem hiding this comment.
Most (or all) of the let in here could be const could you make the switch?
| if (!password) { | ||
| return res.sendStatus(400); | ||
| } | ||
| // For the purpose of this example use httpbin (https://httpbin.org) for the basic authentication request. |
There was a problem hiding this comment.
Could you move the part where you do the Basic auth check in a separate method? Because this will be the part that developers will need to adapt if they want to use this in their own system so it's better in tis own function. And make sure to add a TODO like:
// TODO(DEVELOPER): In production you'll need to update this method so that it sends a Basic auth request to your own credentials system.
| */ | ||
| function createFirebaseAccount(uid) { | ||
| // Create or update the user account. | ||
| const userCreationTask = admin.auth().updateUser(uid, {}).catch(error => { |
There was a problem hiding this comment.
Actually you don't need to update the user here since we're not adding any information here (email, profile pic etc...) so you can just go ahead and generate the custom token and voila. That's because using a custom token on the client will automatically create the Firebase account if it does not exist.
Basically change the whole method by:
function createFirebaseAccount(uid) { // Create a Firebase custom auth token. const token = admin.auth().createCustomToken(uid); console.log('Created Custom token for UID "', uid, '" Token:', token); return token; } username-password-auth/main.css Outdated
| #demo-subscribe-button, | ||
| #demo-unsubscribe-button { | ||
| margin-right: 20px; | ||
| } No newline at end of file |
There was a problem hiding this comment.
Add newline at the end of the file :)
username-password-auth/main.css Outdated
| @@ -0,0 +1,62 @@ | |||
| /** | |||
| * Copyright 2015 Google Inc. All Rights Reserved. | |||
There was a problem hiding this comment.
I don't think this file is used. remove it? The one that is being served is the main.css in the public directory.
| // Initiates the sign-in flow. | ||
| Demo.prototype.signIn = function() { | ||
| var err = this.signInError; | ||
| err.innerText = ""; |
There was a problem hiding this comment.
Can you use single quotes for Strings in JavaScrip in the whole file?
This is handled by the CORS Express middleware
Move basic authentication out into separate function Add error handling
| I have completed all the requested changes + added some extra error handling :) |
| Looks good, thanks so much @beaspider ! |
3 participants
I have a use case where I need to authenticate with a username/password from a 3rd party system.
I have added an example username-password-auth