Moodle
  1. Moodle
  2. MDL-38837

Check for http or https for include in openbadges backpack support.

        Details

        • Type: Bug Bug
        • Status: Closed
        • Priority: Major Major
        • Resolution: Fixed
        • Affects Version/s: 2.5
        • Fix Version/s: 2.5.2
        • Component/s: Badges
        • Testing Instructions:
          Hide

          This should be properly tested as I did not have required set up to do this myself. Testing requires web site to be accessible from the internet.

          When issuer.js from Mozilla Backpack is included, it should be now included through http or https, depending on your install. This can be checked through page source in the browser.

          If Mozilla backpack service is down, it should return a message advising user to try again later. This can be simulated by:

          1. Visiting the my badges page
          2. Opening the developer console and entering "OpenBadges = undefined;" without the quotes
          3. Clicking the "Download to backpack" icon

          For other testing:

          1. Make sure that you have backpack connection set up beforehand.
          2. Export of badges directly to the backpack can be done from two locations (mybadges.php page and issued badge page - badge.php). Try clicking "Add to backpack" from any of those two locations.
          3. If there are no problems with the Mozilla Backpack and user web site is accessible from the Internet, badge should be successfully added to the backpack.
          Show
          This should be properly tested as I did not have required set up to do this myself. Testing requires web site to be accessible from the internet. When issuer.js from Mozilla Backpack is included, it should be now included through http or https, depending on your install. This can be checked through page source in the browser. If Mozilla backpack service is down, it should return a message advising user to try again later. This can be simulated by: Visiting the my badges page Opening the developer console and entering "OpenBadges = undefined;" without the quotes Clicking the "Download to backpack" icon For other testing: Make sure that you have backpack connection set up beforehand. Export of badges directly to the backpack can be done from two locations (mybadges.php page and issued badge page - badge.php). Try clicking "Add to backpack" from any of those two locations. If there are no problems with the Mozilla Backpack and user web site is accessible from the Internet, badge should be successfully added to the backpack.
        • Affected Branches:
          MOODLE_25_STABLE
        • Fixed Branches:
          MOODLE_25_STABLE
        • Pull from Repository:
        • Pull Master Branch:
          MDL-38837_dev
        • Pull Master Diff URL:

          Description

          badges/badge.php

          contains js include for http://backpack.openbadges.org/issuer.js.

          This should be checking for http(s) and including the appropriate js to prevent mixed secure/insecure warnings.

            Gliffy Diagrams

              Issue Links

              Discovered while testing

              Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-40710 Redesign t/backpack

              • Minor - Minor loss of function where workaround is possible
              • Open
              has a non-specific relationship to

              Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-28484 We do not have a function to detect if we are being served on https

              • Minor - Minor loss of function where workaround is possible
              • Closed
              Testing discovered

              New Feature - A new Moodle feature which has yet to be developed. MDL-35073 Open Badges Integration

              • Major - Major loss of function, incorrect output
              • Closed

                Activity

                Ascending order - Click to sort in descending order
                Hide
                Permalink
                Andrew Nicols added a comment -

                Just seen this issue affecting moodle.org

                Show
                Andrew Nicols added a comment - Just seen this issue affecting moodle.org
                Hide
                Permalink
                Simon Coggins added a comment -

                Yes, for me too. It is preventing directly pushing badges to your backpack, because issuer.js is being blocked as insecure content.

                Loading the page gives:

                [blocked] The page at https://moodle.org/badges/badge.php?hash=3281c2f5e7eba2c0b08264ce523047363c96509c ran insecure content from http://backpack.openbadges.org/issuer.js.

                Then clicking the "Download to backpack" button gives a JS error:

                Uncaught ReferenceError: OpenBadges is not defined

                Show
                Simon Coggins added a comment - Yes, for me too. It is preventing directly pushing badges to your backpack, because issuer.js is being blocked as insecure content. Loading the page gives: [blocked] The page at https://moodle.org/badges/badge.php?hash=3281c2f5e7eba2c0b08264ce523047363c96509c ran insecure content from http://backpack.openbadges.org/issuer.js . Then clicking the "Download to backpack" button gives a JS error: Uncaught ReferenceError: OpenBadges is not defined
                Hide
                Permalink
                Yuliya Bozhko added a comment -

                Submitted both 25 and master branch patches. Please let me know if there are any issues with this patch as I couldn't test it completely myself.

                Show
                Yuliya Bozhko added a comment - Submitted both 25 and master branch patches. Please let me know if there are any issues with this patch as I couldn't test it completely myself.
                Hide
                Permalink
                Frédéric Massart added a comment -

                Hi Yuliya,

                thanks for fixing this. Here are a few minor comments after peer reviewing your patch.

                1. Could you add some testing instructions to confirm that it is still possible to export to your Backpack (from all the possible locations)
                2. When using a YUI module, we often place the ID/Classes/selectors in a constant, so that they are easy to spot, update and re-use through the methods. Could you consider doing that for issued-badge-table and friends?
                3. As it seems that #backpack-error is specific to addtobackpack(), perhaps the ID should reflect that instead of being that generic. But that's not really a big deal. My only concern is that we are often just adding some IDs for JS but end up polluting the HTML, and re-using them in our CSS files, which creates dependencies, and the whole thing becomes a mess.
                4. Really minor, but strpos($CFG->wwwroot, 'https://') !== false could test against === 0, which is bulletproof.

                Feel free to push for integration whenever ready!

                Many thanks!
                Fred

                Show
                Frédéric Massart added a comment - Hi Yuliya, thanks for fixing this. Here are a few minor comments after peer reviewing your patch. Could you add some testing instructions to confirm that it is still possible to export to your Backpack (from all the possible locations) When using a YUI module, we often place the ID/Classes/selectors in a constant, so that they are easy to spot, update and re-use through the methods. Could you consider doing that for issued-badge-table and friends? As it seems that #backpack-error is specific to addtobackpack() , perhaps the ID should reflect that instead of being that generic. But that's not really a big deal. My only concern is that we are often just adding some IDs for JS but end up polluting the HTML, and re-using them in our CSS files, which creates dependencies, and the whole thing becomes a mess. Really minor, but strpos($CFG->wwwroot, 'https://') !== false could test against === 0, which is bulletproof. Feel free to push for integration whenever ready! Many thanks! Fred
                Hide
                Permalink
                Yuliya Bozhko added a comment -

                Hi Fred,

                1. Will do.
                2. I was actually going to create a new task to rewrite JS stuff as a module because it is kind of a separate issue.
                3. Will do.
                4. Make sense. I will correct that.

                Thanks! Please let me know what you think about #2 and I will get everything fixed today.

                Yuliya

                Show
                Yuliya Bozhko added a comment - Hi Fred, 1. Will do. 2. I was actually going to create a new task to rewrite JS stuff as a module because it is kind of a separate issue. 3. Will do. 4. Make sense. I will correct that. Thanks! Please let me know what you think about #2 and I will get everything fixed today. Yuliya
                Hide
                Permalink
                Yuliya Bozhko added a comment -

                Oh, wait, I totally misunderstood your second point Never mind. Fixing things now

                Show
                Yuliya Bozhko added a comment - Oh, wait, I totally misunderstood your second point Never mind. Fixing things now
                Hide
                Permalink
                Frédéric Massart added a comment -

                Hi Yuliya, moving this to a YUI module is certainly a good idea. I personally wouldn't mind the IDs to be places in constants at that moment. But if you found a way to emphasise them in the current patch that is even better!

                Show
                Frédéric Massart added a comment - Hi Yuliya, moving this to a YUI module is certainly a good idea. I personally wouldn't mind the IDs to be places in constants at that moment. But if you found a way to emphasise them in the current patch that is even better!
                Hide
                Permalink
                Dan Poltawski added a comment -

                Linking to MDL-28484, which I haven't fixed but would've helped with this.

                Show
                Dan Poltawski added a comment - Linking to MDL-28484 , which I haven't fixed but would've helped with this.
                Hide
                Permalink
                Dan Poltawski added a comment -

                Integrated to master and 25 - thanks Yuliya!

                Show
                Dan Poltawski added a comment - Integrated to master and 25 - thanks Yuliya!
                Hide
                Permalink
                Ankit Agarwal added a comment - - edited

                Everything works as described. I did see the message "There was a problem connecting to your backpack service provider. Please try again later.", following the instructions. However I found it a bit of surprise, when I connected my backpack no verification whatsoever was required. But since that is not a part of this issue. Am passing this.

                Also I find ngrok (https://ngrok.com/) very useful to test features that need callback. Posting it here, so it might be useful to someone else.

                Thanks

                Show
                Ankit Agarwal added a comment - - edited Everything works as described. I did see the message "There was a problem connecting to your backpack service provider. Please try again later.", following the instructions. However I found it a bit of surprise, when I connected my backpack no verification whatsoever was required. But since that is not a part of this issue. Am passing this. Also I find ngrok ( https://ngrok.com/ ) very useful to test features that need callback. Posting it here, so it might be useful to someone else. Thanks
                Hide
                Permalink
                Simon Coggins added a comment -

                Hi Ankit,

                Regarding verification, this is something that we are aware of and have been considering a solution for. I have filed a ticket with some further information here:

                https://tracker.moodle.org/browse/MDL-40848

                Simon

                Show
                Simon Coggins added a comment - Hi Ankit, Regarding verification, this is something that we are aware of and have been considering a solution for. I have filed a ticket with some further information here: https://tracker.moodle.org/browse/MDL-40848 Simon
                Hide
                Permalink
                Ankit Agarwal added a comment -

                Great, it has one more vote now

                Show
                Ankit Agarwal added a comment - Great, it has one more vote now
                Hide
                Permalink
                Damyon Wiese added a comment -

                Thanks again for another week of fixes, improvements and testing. These changes have been released to the world.

                Cheers, Damyon

                Show
                Damyon Wiese added a comment - Thanks again for another week of fixes, improvements and testing. These changes have been released to the world. Cheers, Damyon

                  People

                  • Votes:
                    2 Vote for this issue
                    Watchers:
                    6 Start watching this issue

                    Dates

                    • Created:
                      Updated:
                      Resolved: