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

[WASH-930] TozID login for iOS #13

Merged
merged 2 commits into from
Jul 30, 2021
Merged

[WASH-930] TozID login for iOS #13

merged 2 commits into from
Jul 30, 2021

Conversation

carrala
Copy link
Contributor

@carrala carrala commented Jul 27, 2021

  • Adds login method for iOS, which uses the updated e3db-swift and returns a Client
  • Also updated the permissions in info.plist so that http requests succeed

@carrala carrala changed the title [WASH-390] TozID login for iOS [WASH-930] TozID login for iOS Jul 27, 2021
@sjonz
Copy link

sjonz commented Jul 27, 2021

Just a suggestion - make a follow up task (e.g. Jira Subtask) to make sure we unpin the e3db dependency.

lib/tozny_model.dart Show resolved Hide resolved
@@ -5,21 +5,25 @@ public class E3dbSerializer {
static func realmFromJson(json: String) -> Application {
let jsonData = json.data(using: .utf8)!
let realmConfig: RealmConfigSerializer = try! JSONDecoder().decode(RealmConfigSerializer.self, from: jsonData)
return Application(apiUrl: realmConfig.apiURL, appName: realmConfig.appName, realmName: realmConfig.realmName, brokerTargetUrl: realmConfig.brokenTargetURL)
return Application(apiUrl: realmConfig.apiURL, appName: realmConfig.appName, realmName: realmConfig.realmName, brokerTargetUrl: realmConfig.brokerTargetURL)
Copy link
Contributor

@galxy25 galxy25 Jul 27, 2021

Choose a reason for hiding this comment

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

what was broken is now fixed 🧙🏽‍♀️ 🧙🏽‍♂️

Copy link

@pirtleshell pirtleshell left a comment

Choose a reason for hiding this comment

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

approved assuming there are no concerns around NSAllowsArbitraryLoads

Comment on lines 44 to 48
<key>NSAppTransportSecurity</key>
<dict>
<key>NSAllowsArbitraryLoads</key><true/>
</dict>

Choose a reason for hiding this comment

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

just flagging that according to the docs this configuration flag will need to be justified to Apple to get this code in the app store. i'm not familiar enough with this project to know if that is a concern or if this is just some debug code to allow non-https protocol requests. just noting it may come up

You must supply a justification during App Store review if you set the key’s value to YES, as described in Provide Justification for Exceptions. Use this key with caution because it significantly reduces the security of your app. In most cases, it’s better to upgrade your servers to meet the requirements imposed by ATS, or at least to use a narrower exception.

https://developer.apple.com/documentation/bundleresources/information_property_list/nsapptransportsecurity/nsallowsarbitraryloads

Copy link
Contributor

Choose a reason for hiding this comment

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

updated to add this context for future readers

- Adds login method for iOS, which uses the updated e3db-swift and returns a Client
- Converts E3db Config to FlutterConfig so dart can decode the json.
- Updates permissions to allow navigation to http addresses.
@galxy25 galxy25 merged commit ced9cff into trunk Jul 30, 2021
@galxy25 galxy25 deleted the wash-930-login-identity branch July 30, 2021 21:10
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.

None yet

7 participants