Azure-Samples / ms-identity-python-webapp

A Python web application calling Microsoft graph that is secured using the Microsoft identity platform
MIT License
293 stars 138 forks source link

Edit Profile Not Working for B2C After Login #126

Closed bwhybel closed 8 months ago

bwhybel commented 8 months ago

I have been following the instructions for B2C here. There are a few instances where the instructions are not super clear, but have been able to parse through them nonetheless. However, I'm stuck on utilizing the Edit Profile link on the home page after logging in. I keep seeing this error:

ValueError

ValueError: The web page with complete_log_in() MUST be visited after the web page with log_in()

I'm not sure what the issue is here. Should this:

@app.route(app_config_b2c.REDIRECT_PATH)
def auth_response():
    result = auth.complete_log_in(request.args)
    if "error" in result:
        return render_template("auth_error.html", result=result)
    return redirect(url_for("index"))

actually not be the function definition for the auth response?

rayluo commented 8 months ago

Nah. You shouldn't need to change that route (or anything inside app.py for that matter). The same source code is expected to support B2C, as long as you configured your B2C settings into your .env like this. Does that not work for you?

bwhybel commented 8 months ago

Didn't I have to change this to pull app_config_b2c (and then all the additional calls)?

bwhybel commented 8 months ago

Is it possible my edit profile user flow is incorrectly setup? Didn't seem like there were any requirements when setting it up, but even when i run that user flow from the azure portal, I'm getting that error page

rayluo commented 8 months ago

Didn't I have to change this to pull app_config_b2c (and then all the additional calls)?

Sorry for the confusion. You should follow our README, particularly this section. (FWIW, the external docs that it links to, was also recently updated, specifically for this sample. So, the sample and its doc are in sync and up-to-date.) Let us know how that goes.

BTW, the existing app_config_b2c.py was a relic. We will remove it in our next commit.

bwhybel commented 8 months ago

Going back to that gives me: image when I try to login. I don't see setting up that scope anywhere in the README or the documentation.

rayluo commented 8 months ago

Going back to that gives me: image when I try to login. I don't see setting up that scope anywhere in the README or the documentation.

The README and/or the doc just tell you what steps would be needed to setup your app, such as what settings are needed. But you are the only one who would know the actual values that is suitable to your setup, such as what API your app is going to call, what scope that API would require. So, you would need to substitute the placeholder values inside the .env to your real values.

Does that make sense?

bwhybel commented 8 months ago

I guess I'm confused why all the information about the SCOPE parameter would be included in step 6, but I'm getting this failure on step 5.

rayluo commented 8 months ago

I guess I'm confused why all the information about the SCOPE parameter would be included in step 6, but I'm getting this failure on step 5.

Fair point. We will keep this in mind and improve this part in a future commit.

bwhybel commented 8 months ago

Am I correct that I do need to delete the value from this array in order to get a working solution for part 5?

bwhybel commented 8 months ago

Or are there additional steps i need to do in the B2C tenant for it to work with User.ReadBasic.All?

rayluo commented 8 months ago

Am I correct that I do need to delete the value from this array in order to get a working solution for part 5?

IIRC, that should work.

Or are there additional steps i need to do in the B2C tenant for it to work with User.ReadBasic.All?

The scope User.ReadBasic.All is for Microsoft Graph. I believe B2C does not access MS Graph. You can keep the scopes empty so that your app will do sign-in only. You will do the "call an API" part later when you have your own API in your B2C tenant at that time.

bwhybel commented 8 months ago

thanks, i'm coming into this totally cold, so things not working when I think I'm following the docs gets confusing

bwhybel commented 8 months ago

I'm still not able to get my edit_profile flow to work though

bwhybel commented 8 months ago

should the redirect after editing profile really be calling into

    result = auth.complete_log_in(request.args)

?

It seems like that needs to directly follow a call log_in, but is an edit profile flow going to do that? Because that is just going to this link: https://github.com/Azure-Samples/ms-identity-python-webapp/blob/main/templates/index.html#L17

rayluo commented 8 months ago

Quick question. When that "unable to get my edit_profile flow to work" happens, have you already successfully logged in, and even successfully visited the Edit Profile page? In other words, did the issue only happen when you are returning from the Edit Profile page?

I do not currently have a set of B2C setup for me to reproduce this. If you are interested in sharing a set of test setup to my email (visible in my github profile), I can take a close look in the next several days.

bwhybel commented 8 months ago

Sorry, yes that's unclear. The edit profile flow is working (I get to the generic "edit user details page), but as soon as I click continue on that page, it redirects to me to localhost:5000/getAToken and fails

rayluo commented 8 months ago

UPDATE: Since I do not currently have a B2C setup to reproduce the issue, I'm providing this tentative adjustment for you to test, @bwhybel

Run the following command in your environment, and let me know whether that helps.

pip install git+https://github.com/rayluo/identity.git@fallback

bwhybel commented 8 months ago

so that does work now, I'm just getting this message* now:

We found no prior log_in() info from current session. This situation may be caused by: (1) Special scenarios such as returning from B2C's edit profile, in which case you can simply ignore this warning, or (2) sessions were all reset due to a recent server restart, in which case you can simply restart a new log-in, or (3) the session was stored on the file system of another server, in which case you need to use either a centralized session store or a load balancer with sticky session (a.k.a. affinity cookie).

so what did that package actually change without making any changes to code?

rayluo commented 8 months ago

so that does work now,

Great. Then the fix works. I'll merge that in and cut a new release in the near future.

I'm just getting this message* now:

We found ... may be caused by: (1) Special scenarios such as returning from B2C's edit profile, in which case you can simply ignore this warning, ...

so what did that package actually change without making any changes to code?

If you meant to say "without making any changes to (this sample's) code", then it should not come as a surprise. The sample itself contains very few source code, because most of the heavy-lifting is done by underlying libraries. In this case, an oversensitive hard failure (exception) is now changed to an informative warning. That makes the difference.

rayluo commented 8 months ago

Now fixed in upstream library.