plavjanik commented 4 years ago

Implements all the points of #3.

This PR implements:

[x] Improved authentication error handling and diagnostics
- Wrong credentials are reported as 401 without details if the user ID is valid
- Expired password is reported to the user as 401 with additional details
- Internal authentication error (SAF error or misconfiguration) are reported as 500 and logged as errors to the server log
- Added integration tests
[x] Java API to do authorization checks
[x] Authorization implementation using the Java SAF APIs (DATASET resource class is not in the scope of this PR - https://github.com/zowe/sample-spring-boot-api-service/issues/47)
[x] Check that the application has required permissions for authentication at startup (BPX.SERVER)
[x] Markdown documentation for authorization checks
[x] Spring integration of the security checks for REST APIs

I recommend starting the review by reading the documentation - https://github.com/zowe/sample-spring-boot-api-service/pull/45/files#diff-ba22a8700b747216eee9fc3be67ece71

codecov-io commented 4 years ago

Codecov Report

Merging #45 into master will decrease coverage by 4.19%. The diff coverage is 73.92%.

@@             Coverage Diff             @@
##             master      #45     +/-   ##
===========================================
- Coverage     76.98%   72.79%   -4.2%     
- Complexity       42      230    +188     
===========================================
  Files            21       66     +45     
  Lines           126      838    +712     
  Branches          3       70     +67     
===========================================
+ Hits             97      610    +513     
- Misses           27      194    +167     
- Partials          2       34     +32

Impacted Files	Coverage Δ	Complexity Δ
...ity/authentication/ZosAuthenticationException.java	`75% <ø> (ø)`	`1 <0> (?)`
.../commons/spring/security/MethodSecurityConfig.java	`0% <0%> (ø)`	`0 <0> (?)`
...ring/security/SafMethodSecurityExpressionRoot.java	`0% <0%> (ø)`	`0 <0> (?)`
...ecurity/service/ZosJniPlatformSecurityService.java	`0% <0%> (ø)`	`0 <0> (?)`
...zos/security/platform/SafPlatformClassFactory.java	`0% <0%> (ø)`	`0 <0> (?)`
...g/security/SafMethodSecurityExpressionHandler.java	`0% <0%> (ø)`	`0 <0> (?)`
...s/security/platform/MockPlatformAccessControl.java	`100% <100%> (ø)`	`21 <21> (?)`
...ommons/zos/security/platform/PlatformAckErrno.java	`100% <100%> (ø)`	`4 <4> (?)`
...mmons/zos/security/platform/PlatformErrorType.java	`100% <100%> (ø)`	`1 <1> (?)`
...security/service/DummyPlatformSecurityService.java	`100% <100%> (ø)`	`11 <3> (?)`
... and 59 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b2750d2...b4f6769. Read the comment docs.

dkelosky commented 4 years ago

I tested the two new endpoints on z/OS and got:

and:

And finished reading the document - this is a great feature!

plavjanik commented 4 years ago

final comment - when do the new releases occur? after a merge to master?

Hi Dan, thank you a lot for catching all those problems!

Releases occur when you create a new release on GitHub - https://github.com/zowe/sample-spring-boot-api-service/releases/new.

I tried to describe it at https://github.com/zowe/sample-spring-boot-api-service/blob/master/docs/releasing.md.

Some projects publish a new snapshot version to JFrog OSS when there is a successful master build. We do not do that but might want to. It would be a way how we can provide an artifact to someone who needs a quick fix before we will do a new release.

dkelosky commented 4 years ago

no problems, just trivial, minor things. awesome work - i'm anxious for others to try this out

zowe / sample-spring-boot-api-service

SAF authorization APIs #45

Codecov Report