Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding structures for iot custom authorizer request/response #67

Merged
merged 23 commits into from Sep 18, 2021

Conversation

ynori7
Copy link
Contributor

@ynori7 ynori7 commented Apr 5, 2018

Adding structures to fit to the documentation here: https://docs.aws.amazon.com/iot/latest/apireference/API_TestInvokeAuthorizer.html

@codecov-io
Copy link

codecov-io commented Apr 5, 2018

Codecov Report

Merging #67 (77831cb) into master (b3dd246) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #67   +/-   ##
=======================================
  Coverage   72.22%   72.22%           
=======================================
  Files          19       19           
  Lines         738      738           
=======================================
  Hits          533      533           
  Misses        138      138           
  Partials       67       67           

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 b3dd246...77831cb. Read the comment docs.

@LegNeato
Copy link
Contributor

@bmoffatt any chance we can get this merged?

Copy link
Collaborator

@bmoffatt bmoffatt left a comment

Choose a reason for hiding this comment

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

The request and response types do not match the linked documentation. Its possible that they've drifted since the PR was created

events/iot.go Show resolved Hide resolved
events/iot.go Outdated Show resolved Hide resolved
events/iot.go Outdated Show resolved Hide resolved
@ynori7
Copy link
Contributor Author

ynori7 commented Jan 26, 2021

The request and response types do not match the linked documentation. Its possible that they've drifted since the PR was created

Yeah, it looks like the contract changed a lot since I made these changes. I've updated everything now and addressed all the comments.

@ynori7 ynori7 requested a review from bmoffatt January 26, 2021 09:58
@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2021

Codecov Report

Merging #67 (a6279db) into main (ac2f18e) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #67   +/-   ##
=======================================
  Coverage   71.63%   71.63%           
=======================================
  Files          19       19           
  Lines        1040     1040           
=======================================
  Hits          745      745           
  Misses        228      228           
  Partials       67       67           

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 ac2f18e...a6279db. Read the comment docs.

@ynori7
Copy link
Contributor Author

ynori7 commented Aug 13, 2021

@bmoffatt Could you take another look?

@ynori7 ynori7 closed this Sep 17, 2021
@ynori7 ynori7 deleted the master branch September 17, 2021 21:01
@ynori7 ynori7 restored the master branch September 17, 2021 21:01
@ynori7 ynori7 reopened this Sep 17, 2021
@@ -0,0 +1,10 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Collaborator

Choose a reason for hiding this comment

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

project files need to be removed :)

events/iot.go Outdated
}

type IoTMqttContext struct {
ClientId string `json:"clientId"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

style: general field and type naming conventions should consistently capitalize these initialisms

Id -> ID
HTTP -> HTTP
Tls->TLS
Mqtt -> MQTT

events/policy.go Outdated
@@ -0,0 +1,7 @@
package events
Copy link
Collaborator

Choose a reason for hiding this comment

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

unused file?

Copy link
Collaborator

Choose a reason for hiding this comment

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

going to delete this one before merging, feel free to re-open if this one was needed for something!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it was used before, but not anymore after resolving the most recent merge conflicts since it was added already somewhere else with a different name. Thanks for cleaning up!

to satisfy the linter
@bmoffatt bmoffatt merged commit 0d9038e into aws:main Sep 18, 2021
@rittneje
Copy link
Contributor

@ynori7 @bmoffatt Unfortunately, I think you referenced the wrong request structure. The actual structure is documented here: https://docs.aws.amazon.com/iot/latest/developerguide/config-custom-auth.html I will file a bug.

I guess for some reason the IoT Core developers decided to make the test request totally different from the real request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants