Allow affiliates to link back to a personal website - display website on leaderboard

VERIFIED FIXED in 3

Status

P1
normal
VERIFIED FIXED
7 years ago
3 years ago

People

(Reporter: chelsea, Assigned: mkelly)

Tracking

({qawanted})

unspecified
qawanted

Details

(Whiteboard: u=user c=leaderboard p=2)

Attachments

(3 attachments)

(Reporter)

Description

7 years ago
Please add a field to the Affiliate profile page where Affiliates can add a website URL. This URL can then be shown along with their name on the leaderboard.

If the Affiliate is linking back to an inappropriate site, remove their presence from the leaderboard. 

On the profile field add a box with the following copy:

Website URL:

and then have a question mark like we have for address with the following copy:

This website will appear along with your username on the Affiliates leaderboard and on your profile. 


Thanks!
(Assignee)

Comment 1

7 years ago
Created attachment 629298 [details]
Mockup of link location

The leaderboard is already kind've crowded. I've attached a mockup that might work, although at some point we'd have to cut off the URL (the link would still be clickable, of course).

chelsea: How does this look to you?
(Reporter)

Comment 2

7 years ago
Comment on attachment 629298 [details]
Mockup of link location

would it make sense to have the users name link back to the website if we're worried about crowding?
OS: Mac OS X → All
Hardware: x86 → All
(Assignee)

Comment 3

7 years ago
We could do that as well. Just make their name blue to signify they're a link?
(Reporter)

Comment 4

7 years ago
Perfect.
(Assignee)

Comment 5

7 years ago
Adding sec-review-needed (a bit late, sorry) to make sure this feature is safe. It's only half-implemented, so we'll request an actual review of the code if needed once it's cleared.

The implementation currently does server-side validation to ensure users only enter valid URLs. In addition, the entire site uses CSP with inline JS disabled, so if somehow a javascript URL is entered it won't run.
Keywords: sec-review-needed
Assignee: mkelly → rforbes
Priority: -- → P3
Keywords: sec-review-needed
Whiteboard: [pending secreview][start yyyy-mm-dd][target yyyy-mm-dd]
Product: Websites → Firefox Affiliates
(Reporter)

Comment 6

6 years ago
Hey all, 

So we just got an email from one of our top-five affiliates who is threatening to leave the program if we can't implement this feature soon. They're not interested in swag, they want traffic back. 

They (and others) also asked for this feature shortly after we launched last year, so I understand their frustration. 

Can we add this to the next sprint/make it a P1? This affiliate has driven almost 200K clicks. 

Please let me know what you think.
Thanks!
C
Chelsea:

I think we can implement this soon but we just had our planning meeting, so unless we bump something it won't get in this iteration.

That being said let me confirm whats left to do here so we can estimate how big it is and get any other pre-reqs out of the way.

Mkelly:

According the above it looks like this was started but not finished? Approx much work is left?

It also looks like we need a sec review, was this ever done, if not I can handle that part.
Whiteboard: [pending secreview][start yyyy-mm-dd][target yyyy-mm-dd] → [pending secreview][start yyyy-mm-dd][target yyyy-mm-dd][qa-wanted]
(Reporter)

Comment 8

6 years ago
Thanks Ben. Please let me know how long it would take and what would be up for bumping.
(Assignee)

Comment 9

6 years ago
We still need a sec-review, but it's less a review and more guidance on if security is okay with this kind've feature and what protections they want around it. There's the risk that users could add links to malware or other harmful sites and have them show up on the leaderboard.

If security thinks the risk is low we can just manually check the urls every once in a while to ensure that nothing malicious or illegal is posted, otherwise we can add in a review process whenever a URL is added or changed that would notify chelsea.

Ben: If you could help get that input I'd be able to get this out the door ASAP. :D
Ok I am on it.

Updated

6 years ago
Flags: sec-review+
The sec-review flag is for the security assurance team tracking purposes.
Flags: sec-review+ → sec-review?(rforbes)
Daniel:

Can you let me know the time-frame for guidance on this one (also let me know if a meeting would be helpful).

I ask for a timeframe because it looks like this was moved to "pending secreview" a few months ago and we now need to finish the work on it.
Raymond / Daniel:

Can you provide a time-frame for completion of the review? If you need additional information please contact me.
Raymond / Daniel: can you provide an update?
it seems to me that the affiliates need to login in order to add this link.  If that is the case, there is no issue at all.  We can track if somebody puts up a malicious link.
Ok agreed, I would say we are sec-reviewed and can move forward.

Thanks again.
(Assignee)

Updated

6 years ago
Whiteboard: [pending secreview][start yyyy-mm-dd][target yyyy-mm-dd][qa-wanted] → [pending secreview][start yyyy-mm-dd][target yyyy-mm-dd]
(Assignee)

Updated

6 years ago
Priority: P3 → P1
Whiteboard: [pending secreview][start yyyy-mm-dd][target yyyy-mm-dd] → [pending secreview][start yyyy-mm-dd][target yyyy-mm-dd] u=user c=leaderboard p=2

Updated

6 years ago
Assignee: rforbes → mkelly

Comment 17

6 years ago
Commits pushed to develop at https://github.com/mozilla/affiliates

https://github.com/mozilla/affiliates/commit/f8996e7eebecf0d2e92a96c2c37c54dd3622e1ef
Add personal links to the leaderboard.

Adds a new UserProfile field for storing a link to a user's personal
website, and links to the website when the user is displayed
on the leaderboard.

bug 760631

https://github.com/mozilla/affiliates/commit/104f193b13a60d4acaa505309ba7878988f15500
Fix admin patch and add show website permission.

Switches from using the funfactory admin object specifically
to patching the django.contrib.admin.site object with it
so that permissions and other default admin pages show up
properly.

Adds a new permission that controls who can display
a website link in their place on the leaderboard.

bug 760631
(Assignee)

Comment 18

6 years ago
This is fixed and available on dev, but might not go out with the next push depending on localization.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Comment 19

6 years ago
Verification Steps:

1. Login to a user account that is on the Leaderboard.
2. Add a website URL using the new website URL field on the Edit Profile Page.
3. Verify that your username on the leaderboard links to the URL you entered on the website field.
4. Verify that invalid links aren't accepted by the Website URL field.
5. Log into the admin interface and edit your user account to remove the "Display website on leaderboard" permission.
6. Verify that your name on the leaderboard no longer links to a website even if you have one specified.
7. Create a new account and verify in the admin interface that the user account has the "Display website on leaderboard" permission by default.
Keywords: qawanted
Whiteboard: [pending secreview][start yyyy-mm-dd][target yyyy-mm-dd] u=user c=leaderboard p=2 → u=user c=leaderboard p=2
Target Milestone: --- → 3
Created attachment 672909 [details]
qa - reopened

Reopened:
A couple of questions -- :-D

1. URL field is required - Now that we're sharing an affiliate's website shouldn't we make this field optional. If the affiliate doesn't want an URL associated with them they should be allowed to leave it blank.

2. Why is there a password field in the profile -- what purpose does it serve?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 21

6 years ago
1. Yes, it should be optional, that's a bug (that I thought I had handled). 

2. The password field appears if you're using an account that has a password assigned from the original registration process; try testing on a brand new account and it shouldn't show up.

We keep the old password auth code around because there's at least one locale that doesn't have a BrowserID translation and if we want to support locales that haven't translated BrowserID we need an alternative authentication method.
> The password field appears...
Good to know. Thanks. I'll retest this scenario

I may have found some more errant behavior, adding a website to a profile doesn't appear to have been passed to my username.

Steps to reproduce:
1. add a valid website url to a user account (http://google.com)
2. save the profile changes
3. visit the downloads leaderboard and click on your test accounts name

Expected:
google.com loads

Actual:
Nothing

<ul>
<li>
<span class="download-number">1</span>
<span class="display-name">
<a href=""> m8ttyb </a>
</span>
<span class="download-count">998</span>
</li>
Created attachment 672927 [details]
qa - reopened - no urls

To clarify -- I'm not seeing clickable urls in these two areas.
Blocks: 803105

Comment 24

6 years ago
Commit pushed to develop at https://github.com/mozilla/affiliates

https://github.com/mozilla/affiliates/commit/e234b0ab925b33d71cb5ded3d51dccbcbb0e59c1
Fix bug 760631: Do not link users with blank website fields.

Changes the check for existence of a user's website to check for truthy
values rather than just for values that aren't None.

Updated

6 years ago
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
(Assignee)

Comment 25

6 years ago
The issues with every name being a link should be fixed now. In addition, we haven't been able to reliably reproduce the issues with names not being linked; every time mbrandt reports it, I look at it and it works fine, and then when he looks again it's fine.

The issues with the website being required were most likely because there was whitespace in the field, which is encapsulated by bug 806958 and not a blocker for release.
QA verified on affiliates-dev, the names are correctly linkified to the url the user specifies in their profile.

mkelly is quite likely correct d(^_^)b
> The issues with the website being required were most likely because there was whitespace in the field
Status: RESOLVED → VERIFIED
Flags: sec-review?(rforbes)
Product: Firefox Affiliates → Firefox Affiliates Graveyard
You need to log in before you can comment on or make changes to this bug.