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

Allow .well-known directory (pki validation / letsencrypt) #1724

Merged

Conversation

BarbUk
Copy link
Contributor

@BarbUk BarbUk commented Sep 9, 2023

Questions Answers
Branch? 8.x
Description? Fix nginx example configuration to allow .well-known directory usage, which is used for pki validation or acme / letsencrypt
Fixed ticket? no

@github-actions github-actions bot added the 8.x label Sep 9, 2023
@BarbUk
Copy link
Contributor Author

BarbUk commented Sep 9, 2023

The same modification should be done to the 1.7 branch.
Should I make another PR ?

@matks
Copy link
Contributor

matks commented Sep 11, 2023

The same modification should be done to the 1.7 branch. Should I make another PR ?

Usually we first merge the PR on 8.x then we create another PR and use git cherry-pick to backport the commit

@jokesterfr
Copy link

Hi there!
It's interesting to mention nginx can be bound to let's encrypt, but I'm not sure this is relevant as the default example for PrestaShop.

Don't get me wrong, this improvement could be provided as a secondary integration example, but you never know the will of the user integrating nginx + prestashop. Use cases are very wide, it could be placed behind another reverse-proxy taking care of the HTTPS, or any other http component... Better not document (and maintain) what is not closely relevant to PrestaShop IMO.

Copy link
Contributor

@matks matks left a comment

Choose a reason for hiding this comment

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

I'm not a NGINX expert but I trust @jokesterfr on this topic 👍

The developer documentation of PrestaShop must act as a standard guide for prestashop newcomers so we can only add in it generic and standard information, we can't push side-cases

@BarbUk
Copy link
Contributor Author

BarbUk commented Sep 11, 2023

Letsencrypt is one of the many services that use a well-known dir.

Usage of this resource is documented in RFC 8615.

You document a configuration that explicitly deny usage of this directory. It should not be blocked by default IMHO.

@thomasnares
Copy link
Contributor

Hi @BarbUk, i suggest moving your example to: https://devdocs.prestashop-project.org/8/scale/webservers/nginx/

  • the page "installation" is more about local environment than internet exposed PS instance;
  • the page "Optimize your nginx configuration" is more about going to production, so it makes sense to mention .well-known here.

You could create a heading and show your code snippet 👍

@matks
Copy link
Contributor

matks commented Sep 12, 2023

Great suggestion from @thomasnares

@BarbUk
Copy link
Contributor Author

BarbUk commented Sep 12, 2023

Thanks for the suggestion @thomasnares.
I've updated the PR.

Copy link
Contributor

@thomasnares thomasnares left a comment

Choose a reason for hiding this comment

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

Thanks @BarbUk, i just added some majs on the brands/services

scale/webservers/nginx.md Outdated Show resolved Hide resolved
@matks matks self-requested a review September 12, 2023 12:58
@kpodemski kpodemski merged commit 8145410 into PrestaShop:8.x Sep 13, 2023
1 check passed
@kpodemski
Copy link
Contributor

thank you @BarbUk !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants