Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/password strength meter #101

Closed
wants to merge 8 commits into from
Closed

Feature/password strength meter #101

wants to merge 8 commits into from

Conversation

yindia
Copy link

@yindia yindia commented Dec 13, 2019

Related issue

Proposed changes

Feature request in ory/kratos i.e. Add password strength meter. It can be used across the ory ecosystem

Checklist

  • I have read the contributing guidelines
  • I have read the security policy
  • I confirm that this pull request does not address a security vulnerability. If this pull request addresses a security
    vulnerability, I confirm that I got green light (please contact security@ory.sh) from the maintainers to push the changes.
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation within the code base (if appropriate)

Further comments

Copy link
Member

@aeneasr aeneasr left a comment

Choose a reason for hiding this comment

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

Thank you - the passwordmeter is exclusive to ORY Kratos and not used anywhere else. So this probably belongs into ory/kratos instead.

Also, please run tests (go test ./...) and linting (make lint) before pushing up a new PR, or use the "Draft PR" GitHub feature to indicate that you're still working on this.

// Responses:
// 200: passwordStrength
// 500: genericError
func (h *Handler) PasswordStrength(rw http.ResponseWriter, r *http.Request, _ httprouter.Params) {
Copy link
Member

Choose a reason for hiding this comment

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

This function doesn't do anything, is that on purpose?

)

// RoutesToObserve returns a string of all the available routes of this module.
func RoutesToObserve() []string {
Copy link
Member

Choose a reason for hiding this comment

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

This is nowhere used. Please don't just copy source files from one destination to another and replace the names.

alive := errors.New("not alive")
handler := &Handler{
H: herodot.NewJSONWriter(nil),
VersionString: "test version",
Copy link
Member

Choose a reason for hiding this comment

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

This causes a compile error.

)

func TestPasswordStrengthMeter(t *testing.T) {
alive := errors.New("not alive")
Copy link
Member

Choose a reason for hiding this comment

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

This is a copy of the health test, but we need a different test suite here.

passwordstrengthmeter/handler.go Show resolved Hide resolved
@yindia
Copy link
Author

yindia commented Dec 16, 2019

@aeneasr do you want it in kratos or i put it in ory/x

@aeneasr
Copy link
Member

aeneasr commented Dec 19, 2019

@evalsocket this belongs into kratos :)

@yindia yindia closed this Dec 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants