Page MenuHomePhorge

VPNController
ClosedPublic

Authored by mollekopf on May 30 2023, 8:58 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 17, 10:38 AM
Unknown Object (File)
Wed, Apr 17, 10:38 AM
Unknown Object (File)
Wed, Apr 17, 10:38 AM
Unknown Object (File)
Wed, Apr 17, 10:37 AM
Unknown Object (File)
Wed, Apr 17, 10:37 AM
Unknown Object (File)
Wed, Apr 17, 10:37 AM
Unknown Object (File)
Mon, Apr 15, 4:04 AM
Unknown Object (File)
Fri, Apr 12, 4:43 PM
Subscribers

Details

Summary

Issues a jwt token signed by app.vpn.signing_key

Diff Detail

Repository
rK kolab
Branch
dev/mollekopf
Lint
Lint Errors
SeverityLocationCodeMessage
Errorsrc/app/Http/Controllers/API/V4/VPNController.php:27phpstanMethod App\Http\Controllers\API\V4\VPNController::token() should return Illuminate\Http\Response but returns Illuminate\Http\JsonResponse.
Errorsrc/tests/Feature/Controller/VPNTest.php:100phpstanCall to an undefined method Lcobucci\JWT\Token::claims().
Errorsrc/tests/Feature/Controller/VPNTest.php:101phpstanCall to an undefined method Lcobucci\JWT\Token::claims().
Errorsrc/tests/Feature/Controller/VPNTest.php:104phpstanCall to an undefined method Lcobucci\JWT\Token::claims().
Unit
No Test Coverage
Build Status
Buildable 43109
Build 17288: arc lint + arc unit

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
26

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

30

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

40

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

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

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.