Skip to content

Conversation

@sreynen
Copy link
Contributor

@sreynen sreynen commented Mar 11, 2020

This is on top of #1, just adding HTML and JS files.

Copy link

@schuyler1d schuyler1d left a comment

Choose a reason for hiding this comment

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

So I think I get the general idea here, now that I've seen both PRs.

Sorry comments are distributed between both -- but maybe consider the comments as the questions of someone coming to the codebase for the first time, unfamiliar with the tech (a likely scenario for us).

So I think my toplines (for both PRs) are:

  • document the security model -- (it is clearer with the static page content, but please put it in the readme)

  • switch to HMAC for the key construction/validation

  • Dont' call it 'Static Site' -- that caused a lot of confusion for me as to what I was trying to evaluate.

  • Fail harder/faster on missing campaign/source if they are expected to be required.

  • Maybe say a little about pywell -- and its usecases -- and a quick link or documentation on how to run a dev environment locally (to see the web part work).

  • How do we construct the keys -- is that in this repo as well? If so, then document that in the readme -- what command do I run to create a key?

@schuyler1d
Copy link

Also, ideally, add a small test that just creates and validates a valid key, and then confirms that an invalid one does indeed fail.

@sreynen sreynen requested a review from schuyler1d April 3, 2020 14:07
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.

3 participants