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

Add support for Strings claim #364

Merged
merged 6 commits into from
May 25, 2018

Conversation

stefan-improbable
Copy link
Contributor

This fixes #314

Copy link

@gavinelder gavinelder left a comment

Choose a reason for hiding this comment

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

Let's see what this does

Copy link
Contributor

@gambol99 gambol99 left a comment

Choose a reason for hiding this comment

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

Hi @stefan-improbable ... much appreciated!! .. I left just the one comment.

Also, any chance we can update the README as well please ..

zap.String("email", user.email),
zap.String("resource", resourceURL),
}

Copy link
Contributor

Choose a reason for hiding this comment

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

given Claims is type Claims map[string]interface{} .. we could just and fail quick .. What do you think?

if _, found := user.claims[claimName]; !found {
    r.log.Warn("the token does not have the claim", errFields...)
    return false
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I've added the statement here.

@stefan-improbable
Copy link
Contributor Author

@gambol99 I've added some info to README

@gambol99
Copy link
Contributor

cheers @stefan-improbable ... LGTM

@gambol99 gambol99 merged commit 88e216b into louketo:master May 25, 2018
@stefan-improbable
Copy link
Contributor Author

@gambol99 thanks for the quick review!

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

Successfully merging this pull request may close these issues.

3 participants