Documentation of `RefreshSignInAsync` is misleading

[blowdart]

From @Whathecode on July 30, 2018 12:40

Is this a Bug or Feature request?:

This is a documentation bug report, in the sense that the SignInManager.RefreshSignInAsync documentation is highly misleading.

Description of the problem:

The current documentation on this method states:

Regenerates the user's application cookie, whilst preserving the existing AuthenticationProperties like rememberMe, as an asynchronous operation.

Parameters user The user whose sign-in cookie should be refreshed.

This has lead me and at least one other user down the wrong path, in that we believed we could pass a different user than the currently logged in user to this method in order to refresh their claims (i.e., next time they perform a request). Instead, what happens is the currently logged in user becomes logged in as this other user which was passed to RefreshSignInAsync.

I read the documentation prior to using this call, but could not derive from the current documentation this is what would happen (and I still cannot).

Version of Microsoft.AspNetCore.Mvc or Microsoft.AspNetCore.App or Microsoft.AspNetCore.All:

Microsoft.AspNetCore.All

Copied from original issue: aspnet/Mvc#8172

[blowdart]

From @mkArtakMSFT on July 30, 2018 16:30

Thanks for contacting us, @Whathecode. @javiercn, can you please review the current documentation and see what kind of changes are required there? Thanks!

[blowdart]

From @javiercn on July 30, 2018 16:33

@haok as he’s the owner

[blowdart]

From @HaoK on July 30, 2018 16:39

@blowdart can you move this issue over to the identity repo?

[HaoK]

The documentation is correct, basically this will generate a new claims principal for the user passed in, with the current auth properties for the current cookie, so this could would let you mix an match a different user with the current user's authentication properties (there isn't much stored in auth properties), so I'm not sure why you would want to mix and match as opposed to just calling SignInAsync directly.

        public virtual async Task RefreshSignInAsync(TUser user)
        {
            var auth = await Context.AuthenticateAsync(IdentityConstants.ApplicationScheme);
            var authenticationMethod = auth?.Principal?.FindFirstValue(ClaimTypes.AuthenticationMethod);
            await SignInAsync(user, auth?.Properties, authenticationMethod);
        }

[Whathecode]

The documentation is correct, basically this will generate a new claims principal for the user passed in, with the current auth properties for the current cookie, ...

Except that the documentation does not state "with the current auth properties for the current cookie". I believe it is this omission which has lead me and the other user down the wrong path.

Maybe I'm simply spoiled being used to the excellent documentation of the .NET Core and Framework APIs. 🙂 Just my two cents...

[Tealons]

@HaoK As one of authors of the referenced stack overflow issues, I think you are missing the point. The documentation is correct, but not complete. Like @Whathecode is suggesting, you could clarify the documentation to prevent misinterpretation... I don't understand why you wouldn't want to do that?

[HaoK]

Feel free to submit a PR to tweak the documentation to emphasize the areas which you think are misleading

[Whathecode]

@HaoK I am not in a position to determine whether or not the documentation is correct. The problem was exactly that I could not determine based on the provided documentation what the expected functionality should be.