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

In HTTP-01, modularize the way of presenting the token. #32

Closed

Conversation

yaslama
Copy link

@yaslama yaslama commented Nov 18, 2015

First, with a simple local webserver.

@zakjan
Copy link
Contributor

zakjan commented Nov 19, 2015

LGTM :)

"time"
)

type httpChallengeMethod interface {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be exported so users can write their own (if they want to)? I realize users won't need to use the type, but maybe for documentation purposes...

Or if we don't want a user to write their own, then we can leave this unexported, I guess.

Copy link
Member

Choose a reason for hiding this comment

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

I think we do want the user to be at least be able to write their own.

Copy link
Author

Choose a reason for hiding this comment

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

The commit 61a0912 fixes that

@xenolf
Copy link
Member

xenolf commented Nov 19, 2015

This is looking pretty good. But could you fix the tests? :)

end chan error
}

func NewHttpChallengeWebserver(optPort string) HttpChallengeMethod {
Copy link
Contributor

Choose a reason for hiding this comment

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

For some reason, the word "Webserver" is really bothering me - maybe because it's actually two words. Can we change this to NewHTTPChallengeWebServer instead with a capital S and a capital HTTP?

Copy link
Author

Choose a reason for hiding this comment

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

In this case, httpChallenge can be renamed to HTTPChallenge if we change it to be exported (like I proposed in my remark to #36) ?

@xenolf
Copy link
Member

xenolf commented Nov 21, 2015

I'd like to discuss the following change to the HttpChallengeMethod interface

type HttpChallengeSolver interface {
    PresentToken(domain, token, keyAuth string) error
    CleanUp() error
}

This would get rid of the callback to PresentToken and would establish a clear seperation between "challenges" and "solvers".
See this gist for a quick implementation.

@mholt
Copy link
Contributor

mholt commented Nov 22, 2015

^ I support this suggestion. This PR is definitely on the right track, and I think @xenolf's suggestion is a fine way to improve it.

@xenolf xenolf mentioned this pull request Nov 26, 2015
@xenolf
Copy link
Member

xenolf commented Dec 2, 2015

Any updates on this? :)

@janeczku
Copy link
Contributor

janeczku commented Dec 2, 2015

This is a great suggestion.

The task of whoever implements the interface is to provision the validation resource to an HTTP server. I therefore suggest the following naming:

type HttpChallengeProvisioner interface or type HttpChallengeProvider interface

Also maybe pass an identifier to the cleanup method so the provider doesn't have to store the reference to the created resource:

type HttpChallengeProvider interface {
    PresentToken(domain, token, keyAuth string) error
    CleanUp(token) error
}

@jehiah
Copy link
Contributor

jehiah commented Jan 25, 2016

@xenolf @yaslama i think you can close this now based on #76

@xenolf xenolf closed this Jan 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants