Skip to content
This repository has been archived by the owner on Dec 13, 2022. It is now read-only.

Parameters twisted for GetConsentKey #1697

Closed
whymatter opened this issue Nov 2, 2017 · 2 comments
Closed

Parameters twisted for GetConsentKey #1697

whymatter opened this issue Nov 2, 2017 · 2 comments
Assignees
Labels
Milestone

Comments

@whymatter
Copy link

Hey guys,

on my way though the code which writes into the PersistedGrants table I found these bits of code which seem to be wrong.

The first parameter of the GetConsentKey function is the subjectId and the second one is the clientId. But all invocations of this method pass clientId as first and subjectId as second parameter. This seems wrong to me.

I guess this is not evil but at least not really clean.

private string GetConsentKey(string subjectId, string clientId)
{
return subjectId + "|" + clientId;
}
/// <summary>
/// Stores the user consent asynchronous.
/// </summary>
/// <param name="consent">The consent.</param>
/// <returns></returns>
public Task StoreUserConsentAsync(Consent consent)
{
var key = GetConsentKey(consent.ClientId, consent.SubjectId);
return StoreItemAsync(key, consent, consent.ClientId, consent.SubjectId, consent.CreationTime, consent.Expiration);
}

Mardoxx added a commit to Mardoxx/IdentityServer4 that referenced this issue Nov 3, 2017
We switch impl in GetConsentKey, and also switch param order for calling functions.
This should not break any existing persisted keys.
@brockallen brockallen added bug and removed bug report labels Nov 11, 2017
@brockallen brockallen added this to the 2.0.4 milestone Nov 11, 2017
@brockallen
Copy link
Member

Merged and fixed, thanks!

@brockallen brockallen modified the milestones: 2.0.4, 2.1 Nov 11, 2017
@brockallen brockallen modified the milestones: 2.1, 2.0.4 Nov 20, 2017
@lock
Copy link

lock bot commented Jan 14, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants