edgexfoundry / security-api-gateway

Owner: Security WG
Apache License 2.0
11 stars 10 forks source link

Feature/kong upgrade #55

Closed tingyuz closed 5 years ago

anonymouse64 commented 5 years ago

@tingyuz could you add a description here of the changes you made here to handle the kong bug with using the Host header?

tingyuz commented 5 years ago

@jpwhitemn

  1. This is not real token file, it is just serving as an example. The real token file will be created during the initialization in the shared volume.
  2. Let me switch that version - I was using the old one for testing and should have 1.0.3 in the Edinburgh release.
tingyuz commented 5 years ago

@anonymouse64 Yes I removed the host header totally as I think the bug from kong 1.0.3 is the combination of host header and the service/route/ACL. I updated the readme file to show the sample request from the client that host header is no longer needed.

anonymouse64 commented 5 years ago

@tingyuz I won't be able to review this today (hopefully should be able to get to this week), but please don't merge until I've had a chance to review this. Thanks!

tingyuz commented 5 years ago

@anonymouse64 I made some changes regarding the OAuth2 configuration in the toml file, mainly reduced the configurable items to simplify the configuration. And I am applying global authorization and ACL for all the microservices so that one token created in one service can be applied for all the EdgeX services VS. one token for each service in the previous version.

tingyuz commented 5 years ago

@anonymouse64 update most of the part in the codebase based on your suggestion. i will have another PR to address the httpstatus code issue you mentioned as it spreads out in the code a couple of places, it might be better to have another PR for it.

anonymouse64 commented 5 years ago

@tingyuz thanks, I'll take a look soon. And also that's fine to have another PR to address the http status code issue, just file an issue about it so we don't forget

tingyuz commented 5 years ago

Open issue #57 to track the change of status code per suggestion from @anonymouse64.