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

Check TOS #164

Open
LecrisUT opened this issue Jan 23, 2022 · 2 comments
Open

Check TOS #164

LecrisUT opened this issue Jan 23, 2022 · 2 comments
Labels
feature request Request for new feature or functionality

Comments

@LecrisUT
Copy link

What would you like to have changed?

Either expose ACMEManager.newACMEClientWithAccount, add this initialization in ACMEManager.PreCheck with the same interactive flag, or add a new function altogether to manually InitializeAccount.

Why is this feature a useful, necessary, and/or important addition to this project?

The goal is to have a check whether or not the cached account has accepted the TOS, or if in interative mode, prompt the user to accept/decline with the link to the TOS. Fetching and exposing the Acme meta directory would also be useful. I am nut sure which approach makes more sense in the given flow, but I am leaning towards separating the TOS code in

certmagic/acmeclient.go

Lines 70 to 123 in b6b3db3

// register account if it is new
if account.Status == "" {
if am.NewAccountFunc != nil {
account, err = am.NewAccountFunc(ctx, am, account)
if err != nil {
return nil, fmt.Errorf("account pre-registration callback: %v", err)
}
}
// agree to terms
if interactive {
if !am.Agreed {
var termsURL string
dir, err := client.GetDirectory(ctx)
if err != nil {
return nil, fmt.Errorf("getting directory: %w", err)
}
if dir.Meta != nil {
termsURL = dir.Meta.TermsOfService
}
if termsURL != "" {
am.Agreed = am.askUserAgreement(termsURL)
if !am.Agreed {
return nil, fmt.Errorf("user must agree to CA terms")
}
}
}
} else {
// can't prompt a user who isn't there; they should
// have reviewed the terms beforehand
am.Agreed = true
}
account.TermsOfServiceAgreed = am.Agreed
// associate account with external binding, if configured
if am.ExternalAccount != nil {
err := account.SetExternalAccountBinding(ctx, client.Client, *am.ExternalAccount)
if err != nil {
return nil, err
}
}
// create account
account, err = client.NewAccount(ctx, account)
if err != nil {
return nil, fmt.Errorf("registering account %v with server: %w", account.Contact, err)
}
// persist the account to storage
err = am.saveAccount(client.Directory, account)
if err != nil {
return nil, fmt.Errorf("could not save account %v: %v", account.Contact, err)
}
}

and adding another function to initialize the account (interactively, error if not accepted) or check Account.TermsOfServiceAgreed if already exists.

I will try to write a PR with this idea if you think it's ok.

@LecrisUT LecrisUT added the feature request Request for new feature or functionality label Jan 23, 2022
@mholt
Copy link
Member

mholt commented Jan 23, 2022

Interesting; why do you need this?

@LecrisUT
Copy link
Author

In go-gitea/gitea#18340, I am decoupling the letsencrypt TOS and generalizing to arbitrary ones defined in the directory meta. So I want to link them to the TOS if the default of not accepting TOS is on, and offer a first-time interactive option to read and accept them, similar to PreCheck.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Request for new feature or functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants