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

Weird ios-only SSO cookie bug #2722

Closed
glebtv opened this issue Apr 18, 2019 · 8 comments · Fixed by #2858
Closed

Weird ios-only SSO cookie bug #2722

glebtv opened this issue Apr 18, 2019 · 8 comments · Fixed by #2858

Comments

@glebtv
Copy link

glebtv commented Apr 18, 2019

Submit feature requests to http://www.mattermost.org/feature-requests/. File non-security related bugs here in the following format:

Summary

Gitlab SSO does not work on iOS

Environment Information

  • Device Name: iPhone (any)
  • OS Version: 10+
  • Mattermost App Version: 1.17, master
  • Mattermost Server Version: 5.10.beta

Steps to reproduce

Seems to be happening if mattermost is using a top level domain (no www, no subdomains)

Expected behavior

Gitlab SSO works

Observed behavior

Gitlab SSO shows {"status": "ok"}

Possible fixes

I have debugged my problem down to this line:
https://github.com/joeferraro/react-native-cookies/blob/master/ios/RNCookieManagerIOS/RNCookieManagerIOS.m#L137

currentCookie.domain has "mydomain.ru", and topLevelDomain has ".mydomain.ru" with a dot in front.
If I add a dot to currentCookie.domain, like this it starts to work without any problems:

for(NSHTTPCookie *currentCookie in allCookies) {
    NSString *domainWithDot = [NSString stringWithFormat:@".%@", currentCookie.domain];
    if([currentCookie.domain containsString:topLevelDomain] || [domainWithDot containsString:topLevelDomain]) {
        [cookies setObject:currentCookie.value forKey:currentCookie.name];
    }
}
@enahum
Copy link
Contributor

enahum commented Apr 18, 2019

Hi @glebtv thanks for the report, that seems to be an issue with the library, have you considered submitting a pull request to the library directly?

@glebtv
Copy link
Author

glebtv commented Apr 18, 2019

There is one already:
joeferraro/react-native-cookies#117

Unfortunately, as per the library's readme it is unmaintained.

If you are willing to use a fork, this fixes it:

"react-native-cookies": "github:bamlab/react-native-cookies#ae9b5bbc673f7e7a0adee31fd707fb786dc53b55",

@nlowe
Copy link

nlowe commented Apr 22, 2019

A few of our users were seeing this as well. Updating to the latest version available from the app store resolved it for them.

@arcdigital
Copy link

I'm still having this issue (where it just says status: ok) and I have the latest version of the app (released yesterday).

@enahum
Copy link
Contributor

enahum commented Apr 23, 2019

@arcdigital make sure your server url matches the SiteURL in the system console

@arcdigital
Copy link

It did, couldn't figure out how to fix it so we changed to a subdomain and it works now.

@glebtv
Copy link
Author

glebtv commented Apr 30, 2019

Since the ios part of the cookie library is now pulled into mattermost-mobile for another issue, maybe fix this one there too?

this is a one-line fix, see my first post

@enahum
Copy link
Contributor

enahum commented Apr 30, 2019

@glebtv perhaps you could submit a PR for that?

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 a pull request may close this issue.

4 participants