Skip to content

feat: Hide OS API key by allowing proxy endpoint#241

Merged
DafyddLlyr merged 15 commits intomainfrom
dp/hide-os-key
Dec 21, 2022
Merged

feat: Hide OS API key by allowing proxy endpoint#241
DafyddLlyr merged 15 commits intomainfrom
dp/hide-os-key

Conversation

@DafyddLlyr
Copy link
Copy Markdown
Contributor

@DafyddLlyr DafyddLlyr commented Dec 14, 2022

What does this PR do?

  • Support the use of a proxy via the osProxyEndpoint property
  • This means that we do not need to expose the OS API key when making requests

How is this achieved?

Before making any requests to the OS, we get the URL from ordnanceSurvey.getServiceURL().

If the user supplied an OS API key, we'll query the OS API directly and append their key. This is the exact same functionality as currently exists.

If the user supplies an osProxyEndpoint value, we'll construct the correct service URL, without an API key. It is the end user's responsibility to host and maintain a suitable service to proxy requests (with the correct authentication) to the OS API. Ensuring proper access to the proxy endpoint can be done through CORS/CORP on the proxy. The documentation in this repository outlines how this can be achieved.

Testing

In order to test this, you can run the dev server locally and add the osProxyEndpoint="https://api.planx.dev/proxy/ordnance-survey" property to the my-map or address-autocomplete components. This will also work on the "Deploy Preview" link generated by the PR.

A number of unit test and service / integration tests have also been added to cover the new functionality, but there's plenty more that could be added to cover all scenarios. I think I've captured the main journeys here but please let me know if you think we need more coverage anywhere.

Documentation

Pitsby doesn't allow markup or any rich text (even line breaks) in it's descriptions, so linking back to a richer markdown page seemed helpful here.

@netlify
Copy link
Copy Markdown

netlify Bot commented Dec 14, 2022

Deploy Preview for oslmap ready!

Name Link
🔨 Latest commit d0f0479
🔍 Latest deploy log https://app.netlify.com/sites/oslmap/deploys/63a1e760da0097000936258d
😎 Deploy Preview https://deploy-preview-241--oslmap.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

- Also some improvements to error handling and boolean logic
@DafyddLlyr DafyddLlyr changed the title feat(wip): Hide OS API key by allowing proxy endpoint feat: Hide OS API key by allowing proxy endpoint Dec 20, 2022
@DafyddLlyr DafyddLlyr requested a review from a team December 20, 2022 13:44
@DafyddLlyr DafyddLlyr marked this pull request as ready for review December 20, 2022 13:46
Copy link
Copy Markdown
Member

@jessicamcinchak jessicamcinchak left a comment

Choose a reason for hiding this comment

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

Looking really solid 🎉 I want to come back with fresher eyes at least once more, but here's initial comments/questions. Appreciate the docs, examples and tests here - makes it much easier to review!

Comment thread index.html Outdated
Comment thread docs/how-to-use-a-proxy.md
Comment thread src/components/address-autocomplete/main.test.ts Outdated
Comment thread src/components/my-map/os-layers.ts
Comment thread src/lib/ordnanceSurvey.test.ts
Comment thread src/lib/ordnanceSurvey.ts
@DafyddLlyr DafyddLlyr merged commit 7979850 into main Dec 21, 2022
@DafyddLlyr DafyddLlyr deleted the dp/hide-os-key branch December 21, 2022 10:33
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.

2 participants