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

Fix error on MySQL backend when bsToken null #63

Merged
merged 2 commits into from
Feb 28, 2023

Conversation

4e554c4c
Copy link
Contributor

@4e554c4c 4e554c4c commented Jan 10, 2023

This commit fixes an error when the bootstrap token for a device is NULL. Apple specifies the behaviour of GetBootstrapToken to return an empty body when there is no bootstrap token for the device. This behaviour has been conformed to.

This commit fixes an error when the bootstrap token for a device is NULL. Apple
specifies[0] the behaviour of `GetBootstrapToken` to return an empty body when
there is no bootstrap token for the device. This behaviour has been conformed
to.

[0]: https://developer.apple.com/documentation/devicemanagement/get_bootstrap_token
@4e554c4c 4e554c4c changed the title 'Fix error on MySQL backend when bsToken null Fix error on MySQL backend when bsToken null Jan 12, 2023
@4e554c4c 4e554c4c force-pushed the bstoken branch 3 times, most recently from 8f60298 to 0287471 Compare January 12, 2023 23:14
@4e554c4c 4e554c4c marked this pull request as ready for review January 12, 2023 23:20
Copy link
Member

@jessepeterson jessepeterson left a comment

Choose a reason for hiding this comment

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

So firstly: the actual changes to the storage backend and the service code are 👍. However the tests I think need some adjustment. Notably around including a new dependency (uuid) and how a new device is created.

Because you're creating a new "enrollment" from actual messages like Authenticate you don't need to generate a UUID — it's already in the Authenticate message and you can Resolve() it into a form that you can use in an mdm.Request (like the nanomdm service does for a real incoming enrollment). If you look at the parsed Authenticate.2.plist message that's the same UUID in the code — I'm not sure why I didn't do this myself in the existing test.

I'm also on the fence about (but leaning against) moving the Enrollment test harness stuff to the shared test package. Enrollment and test data I think should be closer to the actual test. Not to mention — are the relative testdata file paths from where the test is executed from or from where the package resides? I suspect the former. Since it's a shared test module the pathing will be indeterminate (though likely quite predicable given the flat-ish nature of the existing modules).

@4e554c4c 4e554c4c force-pushed the bstoken branch 2 times, most recently from fab346a to b07690d Compare February 27, 2023 17:31
This commit refactors the MySQL tests, and puts some common code into
the storage test library. Instead of depending on the "github.com/google/uuid"
library, we grab device UDIDs from our test data.

The pgsql changes should also be refactored to use these changes, but I
think that deserves another PR.
Copy link
Member

@jessepeterson jessepeterson left a comment

Choose a reason for hiding this comment

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

lgtm! Thanks!

@jessepeterson jessepeterson merged commit 0011b0f into micromdm:main Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants