Skip to content

Refactor/laravel quickstart#25

Open
KMilej wants to merge 37 commits into
mainfrom
refactor/laravel-quickstart
Open

Refactor/laravel quickstart#25
KMilej wants to merge 37 commits into
mainfrom
refactor/laravel-quickstart

Conversation

@KMilej

@KMilej KMilej commented Jan 13, 2026

Copy link
Copy Markdown

Summary

  • Complete Laravel demo API with Approov token verification and optional token binding.
  • Expose protected (/token-check, /token-binding, /token-double-binding) and unprotected endpoints.
  • Provide Docker run scripts, .env configuration, and a CLI test harness.
  • Uses the default Laravel framework structure generated by Composer install.

How to run

  • Clone repo.
  • cp .env.example .env.
  • Set APPROOV_BASE64URL_SECRET.
  • approov api -add example.com
  • bash run_server.sh.
  • In another terminal: bash test.sh.

@devenney devenney left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for this, Kamil.

It looks great, just one observation from my side: the logs are not visible to the user. I think this stems from two decisions:

  1. artisan is invoked with --quiet.
  2. The log driver writes the logs to storage/logs/laravel.log but this is not exposed to the user (unless they docker exec and read the logs manually).

Perhaps this is intentional, but I think we should discuss whether we wish to expose such logging to the user while the container is running. I feel it's quite useful to see how the Approov logic is behaving in realtime.

Comment thread config/logging.php
],
'null' => [
'driver' => 'null',
],

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I see the following error in laravel.log:

[2026-02-02 12:24:10] laravel.EMERGENCY: Unable to create configured logger. Using emergency logger. {"exception":"[object] (InvalidArgumentException(code: 0): Driver [null] is not supported. at /app/vendor/laravel/framework/src/Illuminat
e/Log/LogManager.php:225)

It seems that the solution is this:

Suggested change
],
],
'deprecations' => [
'driver' => 'daily',
'path' => storage_path('logs/php-deprecation-warnings.log'),
'level' => 'debug',
'days' => 7,
],

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I made a few changes to improve the logging. If the secret isn’t configured, the server logs now include a clear message. I also enhanced the overall logging to be more professional, for example, capturing what happened, where it happened, and the status code, among other details.

- trim whitespace and wrapping quotes, before placeholder check and base64url decode

- validate APPROOV_BASE64URL_SECRET in scripts/build.sh before docker run (missing, placeholder, empty, invalid base64url) and require base64 CLI

- show container logs when startup container exits early or readiness timeout happens to make docker startup failures easier to diagnose

- map TypeError/ValueError in ApproovTokenVerifier to Approov error codes instead of falling back to generic internal verification errors

- set APP_DEBUG default to false and update README line links plus token-binding responsibility descriptions

@jexh jexh left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hi Kamil, I've had a look through and I think the content is really good. What you have put together here is very impressive. There are a few changes that I would like you to make. I hope these are not things that contradict what Brendan has already asked.

  1. I think the README content is good but I think we need to reorder the sections. I think it should have the following structure:
  • A short intro. Including a very short description of what Approov is: a sentence with a link to the website (in case someone lands in the repo before knowing what Approov is).
  • A short contents list that links to the remaining sections in the file
  • A walk through of how to get the example up and running (you currently have too much explanation up front. Most people will want to get the example up and running and then try to understand it.
  • Then the descriptions of the different components and what they do
  1. We need to group the Approov functionality into a single package if we can. At present most of it is in ApproovApplication.php, then a bit in ApproovTokenVerifier and the rest in the Approov package. Ideally, all Approov files are in a single directory and then they are included from the places they are needed. Approov configuration, functionality, and any dynamic state should be controlled by code in the package. Then the app should call into that code to perform Approov related functionality. This is important so that it is as easy as possible for a developer to integrate the token check into their application. Additionally, when we eventually come to release an Approov package for php, it will be natural for people to switch to importing it from elsewhere.
  2. There need to be comments on each of the php functions that you have defined. Getting AI to create them is easy enough but you need to make sure they correctly describe what the function does, its inputs and its outputs. Comments should be provided in the normal format used for php packages.

@KMilej

KMilej commented Mar 31, 2026

Copy link
Copy Markdown
Author

Fixed all 3 points.

  • Added a short introduction and links, and moved the setup instructions to the top.
  • Updated the component description so it matches the code now.
  • Moved the code to app/approov, so after adding a few lines, it can be used to run the approov token verification.
  • Added PHPDoc comments.

I didn’t add a contents section because both GitHub and IDEs already provide an outline view, so it felt unnecessary, but I can add it if needed.

@KMilej KMilej requested a review from jexh March 31, 2026 16:02
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