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

bring linting and CI/CD uptodate #67

Merged
merged 1 commit into from
Jul 25, 2024
Merged

bring linting and CI/CD uptodate #67

merged 1 commit into from
Jul 25, 2024

Conversation

gedge
Copy link
Contributor

@gedge gedge commented Jul 23, 2024

What

add more validate to avoid having someone clicking on ONS buttons/redirects and being sent somewhere nefarious

How to review

does PR pass tests/sanity

Who can review

!me

@gedge gedge force-pushed the feature/secure-redirect branch 2 times, most recently from 5a5dc2e to b373e11 Compare July 23, 2024 16:16
Copy link
Contributor

@lindenmckenzie lindenmckenzie left a comment

Choose a reason for hiding this comment

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

Some tests for that function please - this approach does lead us open to breakages when we configure and use cross domain, like in staging, but it's a pattern we use elsewhere so seems reasonable.

config/config.go Outdated
Comment on lines 94 to 111
// IsSiteDomainURL is true when urlString is a URL and its host ends with `.`+siteDomain (when siteDomain is blank, uses cfg.SiteDomain)
func IsSiteDomainURL(urlString, siteDomain string) bool {
if urlString == "" {
return false
}
if siteDomain == "" {
siteDomain = cfg.SiteDomain
}
urlObject, err := url.ParseRequestURI(urlString)
if err != nil || !strings.HasSuffix(urlObject.Host, "."+siteDomain) {
return false
}
return true
}
Copy link
Contributor

Choose a reason for hiding this comment

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

could we have some light unit testing for this please?

@gedge gedge force-pushed the feature/secure-redirect branch from e27d582 to 1949479 Compare July 25, 2024 10:13
@gedge gedge merged commit f52841d into develop Jul 25, 2024
5 checks passed
@gedge gedge deleted the feature/secure-redirect branch July 25, 2024 12:33
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