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

submit: add support for checking imat tokens #1

Merged
merged 2 commits into from
Nov 13, 2023
Merged

Conversation

sumnerevans
Copy link
Member

@sumnerevans sumnerevans commented Nov 11, 2023

@sumnerevans sumnerevans marked this pull request as ready for review November 13, 2023 15:26
submit.go Outdated Show resolved Hide resolved
submit.go Outdated Show resolved Hide resolved
return nil
}
}

if p.AppName == "booper" {
if gplaySpamEmailRegex.MatchString(p.Data["user_id"]) {
log.Println("Dropping report from", p.Data["user_id"])
Copy link
Member

Choose a reason for hiding this comment

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

I think we still need this filter

Copy link
Member Author

Choose a reason for hiding this comment

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

Why? It seems like a bad heuristic. What if someone just happens to have a bunch of numbers at the end of their gmail account?

Copy link
Member

Choose a reason for hiding this comment

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

We don't want to be spammed by the google play testers in linear, they're still sending over a dozen bug reports per day

Copy link
Member Author

Choose a reason for hiding this comment

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

I used to have a gmail account with 7 numbers at the end.

I went ahead and added this check back in, but this might be something we want to revisit.

}

// All of iMessage on Android is on beeper.com
apiServerURL, ok := s.cfg.APIServerURLs["beeper.com"]
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that's strictly true, this should probably use the current env api server (then dev/staging booper can be pointed at dev/staging rageshake)

Copy link
Member

Choose a reason for hiding this comment

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

Oh wait, we might not be telling rageshake which env it is in 🤔 I guess hardcoding beeper.com for now is easier

Copy link
Member Author

Choose a reason for hiding this comment

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

Do dev/staging booper even exist? As far as I can tell, everyone is on the same app.

Copy link
Member

Choose a reason for hiding this comment

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

Not sure if there's a switcher, but booper used to be pointed at staging until a week ago or something

@sumnerevans sumnerevans merged commit 86578bd into beeper Nov 13, 2023
1 check passed
@sumnerevans sumnerevans deleted the sumner/be-19796 branch November 13, 2023 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants