[HOLD for payment 2022-06-15][$16000] In iOS login password is saved for web address "expensify.cash" - reported by @Santhosh-Sellavel

[mvtglobally]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Open the app,
2. Logout of the App
3. Now enter your email/phone and password login
4. Save the password for future use with Passwords
5. Now Logout again
6. On the Login page, password suggestion will show as it's for expensify.cash

Expected Result:

Password for staging.new.expensify.com or new.expensify.com

Actual Result:

Password for expensify.cash

Workaround:

unknown

Platform:

Where is this issue occurring?

• iOS

Version Number: 1.1.43-0 Reproducible in staging?: Y Reproducible in production?: Y Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos: Any additional supporting documentation

Expensify/Expensify Issue URL: Issue reported by: @Santhosh-Sellavel Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1647038053525679

View all open jobs on GitHub

[melvin-bot[bot]]

Triggered auto assignment to @CortneyOfstad (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[melvin-bot[bot]]

Triggered auto assignment to @bondydaa (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[melvin-bot[bot]]

Triggered auto assignment to @kadiealexander (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[bondydaa]

probably a hardcoded value somewhere, i see these within the app repo

Expensidev/App (main) $ ack expensify.cash
config/electronBuilder.config.js
31:        bucket: isStaging ? 'staging-expensify-cash' : 'expensify-cash',

ios/NewExpensify/Chat.entitlements
9:      <string>applinks:www.expensify.cash</string>
10:     <string>applinks:staging.expensify.cash</string>
12:     <string>applinks:expensify.cash</string>

.github/workflows/platformDeploy.yml
332:        run: aws s3 cp --recursive --acl public-read "$GITHUB_WORKSPACE"/dist s3://expensify-cash/
336:        run: aws s3 cp --recursive --acl public-read "$GITHUB_WORKSPACE"/dist s3://staging-expensify-cash/

src/libs/Navigation/linkingConfig.js
8:        'https://www.expensify.cash',
9:        'https://staging.expensify.cash',

src/libs/actions/Timing.js
43:        ? `expensify.cash.${eventName}.${secondaryName}`
44:        : `expensify.cash.${eventName}`;

src/libs/actions/Session/index.js
180:    const autoGeneratedLogin = Str.guid('expensify.cash-');

src/CONST.js
21:        IOS: 'https://apps.apple.com/us/app/expensify-cash/id1530278510',

src/pages/workspace/bills/WorkspaceBillsFirstSection.js
60:                            <Text style={[styles.textBlue]}>example.com@expensify.cash</Text>
64:                            text={`${emailDomain}@expensify.cash`}

so I'd start there and see how we save the pw in the keychain.

[kadiealexander]

Posted to Upwork:

Internal: https://www.upwork.com/ab/applicants/1507195337885282304/job-details External: https://www.upwork.com/jobs/~01e464e10f7aa79f5d

[melvin-bot[bot]]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @rushatgabhane (Exported)

[melvin-bot[bot]]

Triggered auto assignment to @luacmartins (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[anaskhraza]

Proposal

I think this issue is related to the linking of the URL in linkingConfig.js. As mentioned by @bondydaa this is possibly related to more than one occurrence of the hardcoded string "expensify.cash". I will debug this problem and make the necessary tweaks in the files so the login web address has been fixed.

Thanks, Muhammad Anas

[rushatgabhane]

@anaskhraza thanks for your proposal! To get a better idea of what we expect from a proposal, I'd suggest you to go through these closed issues here for reference.

I will debug this problem and make the necessary tweaks in the files so the login web address has been fixed.

Could you please post what exact occurrences of "expensify.cash" you intend to change? and why were they not changed in the first place?

[parasharrajat]

Leaving a note: these expensify.cash strigs are necessary. I know as I made the change last time.

If you open the expensify.cash it redirects to the new and to keep old URLs working they were left as it is.

so to debug this one should

1. Find out how Android saves passwords and which settings are used for that purpose?
2. How can tweak that setting without breaking? Does Android provide more complex configuration options to manage that?

[kadiealexander]

Adding @jliexpensify to keep an eye on this while I'm away for the next few days, in case we need to hire anyone.

[kadiealexander]

Price doubled!

[luacmartins]

Still looking for proposals

[kadiealexander]

Price doubled again!! Get in while it's hot!!

[izhan]

I was unable to repro on iOS 15.4.1 + Safari. Any specific browser / etc? And I'm assuming this is iCloud Autofill password?

[rushatgabhane]

@izhan maybe give Android a try?

[izhan]

@rushatgabhane ah but the PR screenshot / description is for iOS, no?

[rushatgabhane]

Yes, the screenshot is for the iOS native app. But I can also repro this bug on Android

[izhan]

ahh i'm dumb, thank you that makes much more sense :)

[rushatgabhane]

@Expensify/contributor-plus feel free to pick this issue up, I'm unassigning because I'll be OOO. No untriaged proposals yet. Thanks!

[melvin-bot[bot]]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @rushatgabhane (Exported)

[melvin-bot[bot]]

Current assignee @luacmartins is eligible for the Exported assigner, not assigning anyone new.

[melvin-bot[bot]]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat (Exported)

[melvin-bot[bot]]

Current assignee @luacmartins is eligible for the Exported assigner, not assigning anyone new.

[melvin-bot[bot]]

📣 @mananjadhav You have been assigned to this job by @michaelhaxhiu! Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

[mananjadhav]

Went through the history, waiting for proposals.

[kadiealexander]

Price doubled! 🎉

[luacmartins]

Still looking for proposals!

[kadiealexander]

Price doubled again! 🎉

[aswin-s]

Looks like this is because of the associated domains configuration for the app. For configuring universal links, below domains are linked with the app

https://github.com/Expensify/App/blob/724ccb46ca4b2f48f6052592cbacc7ffe9528c5c/ios/NewExpensify/Chat.entitlements#L8-L14

In this list expensify.cash comes first and naturally that domain is given highest priority out of the associated domains.

Proposal

Rearrange the list of domains so that new.expensify.com domains comes first followed by staging.new.expensify.com.

    <array>
        <string>applinks:new.expensify.com</string>
        <string>applinks:staging.new.expensify.com</string>
        <string>applinks:www.expensify.cash</string>
        <string>applinks:staging.expensify.cash</string>
        <string>applinks:expensify.cash</string>
    </array>

Unfortunately contributors can't test this at our end as it requires the developer to be part of Team 368M544MTT to make this change and rebuild the app.

[luacmartins]

@aswin-s I tried to test your proposal, but I don't think that it worked! Here are the steps that I followed:

1. Deleted the app and any saved passwords from the simulator
2. Updated Chat.entitlements as you suggested
3. Confirmed the changes in Xcode > NewExpensify > Signing & Capabilities > Associated Domains
4. Rebuilt the app
5. Logged in and saved the password
6. Verified that it stills saves under expensify.cash

Let me know if I missed anything in my testing steps.

[aswin-s]

@luacmartins I believe you did a clean build. Also is it possible to test this on a real device? Another test I would like to perform is try removing expensify.cash from associated domains and try a build.

Also while validating apple-app-site-association file at new.expensify.com I see couple of warnings. We are also missing the webcredential key in the apple-app-site-association file as well as in Chat.entitlements file. This would enable users to use the same keychain password across app and mobile web.

https://developer.apple.com/documentation/xcode/supporting-associated-domains.

[luacmartins]

Also is it possible to test this on a real device?

We certainly could, but is the hope that the real device would behave differently in this case?

Another test I would like to perform is try removing expensify.cash from associated domains and try a build.

I tried removing them and it seems to fix the issue. However, as @parasharrajat mentioned here, this is not a viable solution.

We are also missing the webcredential key in the apple-app-site-association file as well as in Chat.entitlements file

I tried to test adding these, but it didn't seem to have any effect 😞 Maybe I'm holding something wrong.

Also while validating apple-app-site-association file at new.expensify.com I see couple of warnings

I'm not sure if these warning provide anything useful here. They seem alright to me.

Are we sure that the order dictates the priority here?

[mananjadhav]

@aswin-s Did you get a chance to go through @luacmartins' questions?

[aswin-s]

We certainly could, but is the hope that the real device would behave differently in this case?

Yes, it is best to test these features on real device as network settings and simulator configs tend to interfere with universal link configurations

I tried removing them and it seems to fix the issue. However, as @parasharrajat mentioned here, this is not a viable solution.

This was suggested just to confirm that app was the build changes were actually getting reflected and picked up by iOS. Not as a fix to the issue.

I tried to test adding these, but it didn't seem to have any effect 😞 Maybe I'm holding something wrong.

webcredential key needs to be added to both Chat.entitlements and the apple-app-site-association file hosted at the specified domains. Only then it would work. It could be verified by downloading and opening below link in a text editor.

https://new.expensify.com/apple-app-site-association

Are we sure that the order dictates the priority here?

Associated domains are the only configuration which associated the iOS app with a web domain. Since I cannot build and test this on my own, it is purely a guess and not a definitive answer.

[luacmartins]

Yes, it is best to test these features on real device as network settings and simulator configs tend to interfere with universal link configurations

That's fair! However, given that we can reproduce this and that removing those entries fixes the issue on the simulator, I'm not convinced that testing on a real device will yield a different outcome. I'll try to get around and test on a real device some time though.

This was suggested just to confirm that app was the build changes were actually getting reflected and picked up by iOS. Not as a fix to the issue.

👍

webcredential key needs to be added to both Chat.entitlements and the apple-app-site-association file hosted at the specified domains. Only then it would work. It could be verified by downloading and opening below link in a text editor.

Here's what I did:

1. Added <string>webcredentials:new.expensify.com</string> here
2. Added the block below before this line

   "webcredentials": {
       "apps": ["368M544MTT.com.chat.expensify.chat"]
   },

3. Clean built the app

Did I miss anything?

Associated domains are the only configuration which associated the iOS app with a web domain. Since I cannot build and test this on my own, it is purely a guess and not a definitive answer.

Thanks for clarifying. Yea, I'm stumped on this issue and how it's picking that particular domain.

[aswin-s]

Did I miss anything?

@luacmartins For webcredentials to work, changes made to apple-app-site-association needs to be deployed to the domains specified in chat entitlements. Simply taking a new build will not do. When app gets installed in device iOS tries to fetch the site-association file from the specified domains and validates the app id. That's how it verifies that credentials could be reused across website and app.

Deploy the well-known folder and check whether apple cache got updated at the below URL. It might take some time to get the CDNs updated with latest version of apple-app-site-association file

https://app-site-association.cdn-apple.com/a/v1/new.expensify.com https://app-site-association.cdn-apple.com/a/v1/staging.new.expensify.com https://app-site-association.cdn-apple.com/a/v1/expensify.cash

[luacmartins]

Hmm that's interesting. I'm testing this on dev so I'm hitting our VM host and not our domains. I think testing it on our domains will be tricky. Any other suggestions?

[aswin-s]

Nope. iOS shares user credentials across the app and website using this setting. So there is no way to bypass this check. Otherwise this would easily be misused by fake apps to steal user credentials claiming to be the original app.

[luacmartins]

Still looking for proposals.

[aswin-s]

@luacmartins I had another look at the issue and it seems that we can safely remove expensify.cash from entitlements file, as expensify.cash does a 301 http redirect to new.expensify.com. So universal links will work even without explicitly adding expensify.cash to associated domains.

This could be verified by removing below lines from Chat.entitlements and taking a new build.

<string>applinks:www.expensify.cash</string>
<string>applinks:staging.expensify.cash</string>
<string>applinks:expensify.cash</string>

Then try to open a link to expensify.cash. If you are trying this on simulator, use below command to send url to simulator. Safari will prompt to open the link in New Expensify app.

xcrun simctl openurl booted https://expensify.cash

Now that we've removed references to expensify.cash from associated-domains, passwords will be saved against expensify.com. I've verified this with another iOS app that I'm managing and seems to work properly.

[luacmartins]

Seems like we can remove those after all... @mananjadhav any concerns about the proposal?

[mdneyazahmad]

Proposal

I totally agree with @aswin-s 's proposal but I thnk we are missing the environment consideration there. Taking a look at the expected result. I think, it should be different for staging and production build. Whatever I am posting here is just a guess and is not tested on my side as @aswin-s stated.

Unfortunately contributors can't test this at our end as it requires the developer to be part of Team 368M544MTT to make this change and rebuild the app.

We need to do these things.

1. Create separate target for (dev, staging and prod).
2. Production target will have new.expensify.com as the preferred domain (1st entry).
3. Staging target will have staging.new.expensify.com as the preferred domain (1st entry).
4. Add webcredentials in apple-app-site-association

    "webcredentials": {
        "apps": ["368M544MTT.com.chat.expensify.chat"]
    },

If we want, we can create a separate bundle id for dev and staging eg(com.chat.expensify.chat.dev, com.chat.expensify.chat.staging) this way we will be able to install both apps at the same time.

Tests:

In all tests consider there is no password set against expensify.com.

Production App

1. Open the app
2. Enter email or phone and continue
3. If saved, it should display the password for new.expensify.com.
4. Enter the password
5. It should prompt to save the password and if saved it should be in new.expensify.com

Staging App

1. Same as production from 1 - 2
2. If saved, it should display the password for staging.new.expensify.com.
3. Enter the password
4. It should prompt to save the password and if saved it should be in staging.new.expensify.com

Please, correct me if anything in my steps is not the expected behaviour. I added this steps as per my understanding.

I am also not sure whether it works as expected in android.

More about multiple environment configuration here https://www.bigbinary.com/blog/handling-environment-specific-configurations-in-react-native

[kadiealexander]

@mananjadhav could you please take a look at the above suggestions?

[aswin-s]

Since both staging and prod app are sharing the same backend, credentials doesn't differ across environment right now. If we need separate credentials for each environment then entitlements file needs to be modified at build time to include staging domain for staging build and prod domains for prod build. There are Fastlane plugins which supports this during build time and works even on CI environments.

But the question here is should we do that. The original issue is just to fix the domain against which credentials are getting saved right now. If we really need environment specific configurations and build time customisation, that should be raised as a separate issue altogether.

[mananjadhav]

Went through the history @luacmartins. @aswin-s's proposal looks good to me.

and I agree we can tackle domain-wise entitlements as a separate issue.

🎀 👀 🎀 C+ reviewed

[aswin-s]

Applied for the job in Upwork @kadiealexander

[kadiealexander]

Thanks Aswin! We just need @luacmartins to take a look first :)

[luacmartins]

@aswin-s your proposal looks good!

[melvin-bot[bot]]

📣 @aswin-s You have been assigned to this job by @luacmartins! Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖