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.

Sign up for GitHub

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

Jump to bottom

process certificate expiration #540

Merged
merged 3 commits into from
Apr 8, 2023
Merged

process certificate expiration #540

merged 3 commits into from
Apr 8, 2023

Conversation

SerpentBytes
Copy link
Contributor

@SerpentBytes SerpentBytes commented Apr 7, 2023 •
edited
Loading

Description

Resolves #528 by deleting expired certificates from the database and sending an email notification.

Steps to test

  • Set the job to repeat every 5 seconds
  • Run the project
  • Request a certificate
  • Run npm run studio and set the validTo date in the past
  • Confirm the expired certificate is deleted from the DB
  • Navigate to http://localhost:8025 to see the email notification:

list view

@SerpentBytes SerpentBytes added category: notifications A service to notify users about their certificates/domains category: certificates Creating certificates, encryption labels Apr 7, 2023
@SerpentBytes SerpentBytes requested review from humphd and a user April 7, 2023 13:35
@SerpentBytes SerpentBytes marked this pull request as ready for review April 7, 2023 13:35
@SerpentBytes SerpentBytes added this to the Milestone 0.9 milestone Apr 7, 2023
Eakam1007
Eakam1007 requested changes Apr 7, 2023
View reviewed changes
app/queues/common/expiration-request.server.ts Outdated Show resolved Hide resolved
app/queues/common/expiration-request.server.ts Outdated Show resolved Hide resolved
Eakam1007
Eakam1007 requested changes Apr 7, 2023
View reviewed changes
Copy link
Contributor

@Eakam1007 Eakam1007 left a comment

Choose a reason for hiding this comment

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

I think if they can't even see the expired records/certificate, renew is kind of misleading as well. Maybe we can remove the entire sentence?

@SerpentBytes SerpentBytes marked this pull request as draft April 7, 2023 20:32
@SerpentBytes SerpentBytes requested a review from a user April 7, 2023 22:38
@SerpentBytes
Process certificate expiration
cc281ad 
 * Initial code to process certificate expiration
 * Add logic to process cert expiration/send notification
 * Make email subject and message more meaningful
 * fix typo
 * fix typo
 * Substitute 'manage' with 'renew'
 * Uncomment code
 * Remove the 2nd sentence
 * Notify about successfully issue certs
 * Uncomment line
@SerpentBytes
Copy link
Contributor Author

@dadolhay I have modified the code to notify only about the most recent and successfully issued certificate.

@SerpentBytes SerpentBytes marked this pull request as ready for review April 8, 2023 14:27
ghost
ghost previously approved these changes Apr 8, 2023
View reviewed changes
humphd
humphd requested changes Apr 8, 2023
View reviewed changes
Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

One question and this is good.

app/queues/common/expiration-request.server.ts Outdated Show resolved Hide resolved
@SerpentBytes SerpentBytes merged commit a4674ae into DevelopingSpace:main Apr 8, 2023
@SerpentBytes SerpentBytes deleted the issue-528 branch April 8, 2023 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@humphd humphd humphd approved these changes

@Eakam1007 Eakam1007 Eakam1007 approved these changes

Assignees

@SerpentBytes SerpentBytes

Labels
category: certificates Creating certificates, encryption category: notifications A service to notify users about their certificates/domains
Projects
None yet
Milestone
Milestone 0.9
Development

Successfully merging this pull request may close these issues.

Delete expired certificates automatically
3 participants
@SerpentBytes @humphd @Eakam1007