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

Add response_mode=query support for OpenID Connect #279

Merged
merged 1 commit into from
Jul 15, 2015
Merged

Add response_mode=query support for OpenID Connect #279

merged 1 commit into from
Jul 15, 2015

Conversation

kevinchalet
Copy link
Contributor

if (!string.IsNullOrWhiteSpace(message.IdToken) ||
!string.IsNullOrWhiteSpace(message.Token))
{
Logger.LogError("An OpenID Connect response cannot contain an identity token " +
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll move the hardcoded message to the .resx file when we're done with the other changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Eilon is moving logging messages to .resx still the recommended approach?

@brentschmaltz
Copy link
Contributor

I good with this PR, ack.

return null;
}
}

Copy link
Member

Choose a reason for hiding this comment

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

nit: no empty line before an else statement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, thanks.

@Tratcher
Copy link
Member

Looks good. Could use some tests.

@kevinchalet
Copy link
Contributor Author

@Tratcher yup, I agree, but the new OpenID Connect tests are a bit cryptic for me, I'm not sure where I'm supposed to add these tests.

@brentschmaltz a suggestion? 😄

@Eilon Eilon added this to the 1.0.0-beta6 milestone Jun 4, 2015
@Eilon
Copy link
Member

Eilon commented Jun 4, 2015

@Tratcher please merge when ready.

@Tratcher Tratcher self-assigned this Jun 4, 2015
@kevinchalet
Copy link
Contributor Author

@Eilon right now, merging this PR is pointless as it depends on #258, which doesn't seem to be ready yet.

// Omitting the response_mode parameter when it already corresponds to the default
// response_mode used for the specified response_type is recommended by the specifications.
// See http://openid.net/specs/oauth-v2-multiple-response-types-1_0.html#ResponseModes
if (!string.Equals(Options.ResponseType, OpenIdConnectResponseTypes.Code, StringComparison.Ordinal) ||
Copy link
Contributor Author

Choose a reason for hiding this comment

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

OpenIdConnectResponseTypes.CodeOnly has been renamed to OpenIdConnectResponseTypes.Code.

AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet#156

@Tratcher
Copy link
Member

@PinpointTownes I think all the PRs you need went in. Are you waiting on anything else? If not then please rebase.

@kevinchalet
Copy link
Contributor Author

@Tratcher I had planned to move the warning message to the .resx file, but according to @Eilon, this doesn't seem to be the appropriate approach anymore.

I'll rebase it when back home (you can cherry pick the unique commit if it's urgent)

@kevinchalet
Copy link
Contributor Author

@Tratcher rebased. As expected, there was no conflict 😄

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants