Skip to content

Orders don't have a "deactivated" status.#301

Merged
aarongable merged 5 commits into
mainfrom
orders-not-deactivated
Mar 7, 2025
Merged

Orders don't have a "deactivated" status.#301
aarongable merged 5 commits into
mainfrom
orders-not-deactivated

Conversation

@jsha

@jsha jsha commented Feb 24, 2020

Copy link
Copy Markdown
Contributor

Use "invalid" instead, and add comments linking to the spec.

Fixes #300

Use "invalid" instead, and add comments linking to the spec.
@cpu

cpu commented Feb 25, 2020

Copy link
Copy Markdown
Contributor

👋 FWIW Boulder has the same divergence: https://github.com/letsencrypt/boulder/blob/9a86abc1550425a73f912c25a11c29160f58a85a/sa/sa.go#L1218-L1221

@jvanasco

Copy link
Copy Markdown
Contributor

I think I managed to still trigger this. I need to do a few more tests to make sure i'm running the right code - I'm not very good with go

@jsha

jsha commented Feb 27, 2020

Copy link
Copy Markdown
Contributor Author

Ok, thanks so much for testing! If you need any help, let me know.

Zola-github
Zola-github previously approved these changes Feb 28, 2020
@jvanasco

Copy link
Copy Markdown
Contributor

I finally got a chance to test. This works as expected from the RFC.

Comment thread core/types.go Outdated

@beautifulentropy beautifulentropy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Comment nits.

Comment thread core/types.go Outdated
Co-authored-by: Samantha <hello@entropy.cat>
@beautifulentropy beautifulentropy self-requested a review May 10, 2022 22:21
@cpu

cpu commented Mar 7, 2025

Copy link
Copy Markdown
Contributor

@aarongable @beautifulentropy @mcpherrinm I bumped into this in some client tests and was wondering if one of you could merge this 🙇

@jvanasco

jvanasco commented Mar 7, 2025

Copy link
Copy Markdown
Contributor

I bumped into this in some client tests and was wondering if one of you could merge this

I now consider this a feature - not a bug - helping me expect/handle broken or malformed responses ;)

@aarongable

Copy link
Copy Markdown
Contributor

Lol, no idea why this never got merged. While pebble is meant to implement ACME differently than Boulder, it isn't meant to implement ACME incorrectly. Thanks for the prod!

@aarongable aarongable merged commit dc9bf9d into main Mar 7, 2025
@aarongable aarongable deleted the orders-not-deactivated branch March 7, 2025 17:27
kwatson added a commit to kwatson/letsencrypt-pebble that referenced this pull request Jun 9, 2025
* 'main' of https://github.com/letsencrypt/pebble: (35 commits)
  add overriding of ARI response (letsencrypt#501)
  wfe: fix a race in `orderForDisplay` (letsencrypt#500)
  Bump golang.org/x/ dependencies (letsencrypt#499)
  currectly triggers BadSignatureAlgorithmProblem at JWS parse time (letsencrypt#492)
  use newer validation subdomain for dns-account-01 (fix CI eggsampler/acme error) (letsencrypt#498)
  Orders don't have a "deactivated" status. (letsencrypt#301)
  Update golangci-lint (letsencrypt#488)
  build(deps): bump github.com/go-jose/go-jose/v4 from 4.0.4 to 4.0.5 (letsencrypt#487)
  Truncate ARI timestamps to millisecond resolution (letsencrypt#485)
  return logical and compliant ARI windows for expiring certs (letsencrypt#484)
  Update dependencies (letsencrypt#481)
  docs: rm mention of subproblems being unimpl'd (letsencrypt#479)
  Fix(NOISSUE): Fix docker compose file example in README.md (letsencrypt#475)
  Add support for ACME Profiles (letsencrypt#473)
  Simplify KU, EKU, and SKID fields of issued certs (letsencrypt#472)
  Update golangci-lint to 1.60.2 (letsencrypt#474)
  Update /x/net for compatibility with go1.23 (letsencrypt#470)
  Reject extra command line args and fix README invocation (letsencrypt#467)
  Document exposing API and management ports when not using docker-compose.yaml (letsencrypt#465)
  Implement latest draft-ietf-acme-ari spec (letsencrypt#461)
  ...
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.

wrong implementation of ACME spec: Orders should transition to "invalid", not "deactivated"

6 participants