Page MenuHomePhorge

VPNController
ClosedPublic

Authored by mollekopf on May 30 2023, 8:58 AM.
Tags
None
Referenced Files
F11736912: D4340.diff
Mon, Apr 15, 4:04 AM
Unknown Object (File)
Fri, Apr 12, 4:43 PM
Unknown Object (File)
Fri, Apr 12, 5:09 AM
Unknown Object (File)
Wed, Apr 3, 1:18 PM
Unknown Object (File)
Wed, Apr 3, 1:18 PM
Unknown Object (File)
Wed, Apr 3, 1:18 PM
Unknown Object (File)
Wed, Apr 3, 1:18 PM
Unknown Object (File)
Wed, Apr 3, 1:18 PM
Subscribers

Details

Summary

Issues a jwt token signed by app.vpn.signing_key

Diff Detail

Repository
rK kolab
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

mollekopf created this revision.
mollekopf added a reviewer: Restricted Project.
machniak added inline comments.
src/composer.json
38

Bump to ^5.0 is available

src/tests/Feature/Controller/VPNTest.php
55

See suspicious linting errors.

src/tests/Feature/Controller/VPNTest.php
55

I have this usage from https://lcobucci-jwt.readthedocs.io/en/latest/quick-start/ ((under "Parsing tokens"), not sure what's up with the linting, but I'll have to test the signature verification indeed.

mollekopf edited the summary of this revision. (Show Details)

Asymmetric crypto, properly validate the signature.

machniak requested changes to this revision.Jun 2 2023, 3:39 PM
machniak added inline comments.
src/app/Http/Controllers/API/V4/VPNController.php
27

This check is redundant. The route requires authentication. I'd remove this. It will fix the linting issue.

31

I'd like to see the option described in config/app.php. Also, should we throw an exception when it's not set?

41

Do we really not want to return JSON to be consistent with the whole API?

src/tests/Feature/Controller/VPNTest.php
101

These linting errors are probably JWT issues, but we should silence them. Maybe telling phpstan what exactly $token is (Lcobucci\JWT\UnencryptedToken?) would fix it.

This revision now requires changes to proceed.Jun 2 2023, 3:39 PM
This revision is now accepted and ready to land.Jun 2 2023, 7:06 PM
This revision was automatically updated to reflect the committed changes.