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 for customizing the size of generated comment IDs #9

Merged
merged 3 commits into from
Jan 18, 2021

Conversation

hispanic
Copy link
Owner

This addresses eduardoboucas#401 - "Allow for customizing the size of generated comment IDs"

Two new config options have been added to the JSON-based config:

commentIdGenerator: {
  doc: 'The scheme to use for generating IDs for comments. More info about nanoid - https://github.com/ai/nanoid',
  format: ['uuid', 'nanoid', 'nanoid-lowercase'],
  default: 'uuid',
  env: 'COMMENT_ID_GENERATOR'
}
commentIdLength: {
  doc: 'The desired length of generated comment IDs. Only applicable when commentIdGenerator is set to "nanoid" or "nanoid-lowercase"',
  format: Number,
  default: 21,
  env: 'COMMENT_ID_LENGTH'
}

Currently, Staticman creates all comment IDs as UUIDs. While using UUIDs is a fool-proof solution that basically guarantees the uniqueness of IDs, it can be overkill. Consider a small blog that receives, at most, one comment each day/week/month/etc. For many/most users, the nature of the IDs that Staticman generates will not matter. However, the Staticman configuration allows for the generated ID to be exposed, when desired. For example, it can be incorporated into the filenames generated by Staticman.

For usage scenarios such as these, it is desirable to be able to configure Staticman to generate shorter/simpler comment IDs. Nano ID has been added to achieve this.

For example, if commentIdGenerator is set to nanoid-lowercase and commentIdLength is set to 8, IDs akin to the following will be generated: ry6-bt9d

According the the Nano ID Collision Calculator, for a blog receiving one comment submission every hour, "~34 years [would be] needed, in order to have a 1% probability of at least one collision".

Michael Harry Scepaniak added 3 commits January 17, 2021 14:54
…equest header, testing it against expected values.

- This commit will be reversed.
…ils to satisfy the CORS configuration, as enabled using the expressjs/cors module, the request will _not_ be rejected by Express. It will simply cause the CORS response header(s) to not be set on the response. As a result, a CORS-compliant browser will raise an error such as the following upon receipt of the response:

"Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at http://localhost:8082/v3/entry/gitlab/ihispanic/staticman-test/master/comments. (Reason: CORS header ‘Access-Control-Allow-Origin’ missing)."

"The CORS specification was never designed as a security mechanism to prevent routes from being called; it was only designed as a mechanism to allow a hole to be added to the cross-origin rules of browser user agents."
- expressjs/cors#109 (comment)

In practice...
- Using CORS to restrict the requesting origin would prevent another site from pointing their POST requests at my Staticman instance. But, if the submissions were high-quality, what reason would I have to complain? And low-quality submissions could just as easily come through via my site. Furthermore, those requests would still be processed by Staticman.
- Using CORS to restrict the requesting origin would not prevent a truly malicious actor from submitted POST requests using a tool (e.g., Postman, etc.) that allows for the Origin header to be spoofed.

Restricting POST requests to a Staticman instance, therefore, comes down to use of express-brute, Akismet, captchas, and the like.
…e size of generated comment IDs"

- eduardoboucas#401
- Allow for Nano ID to be used to customize the length and content of generated comment IDs.
@hispanic hispanic merged commit 0c5ad3c into dev Jan 18, 2021
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.

1 participant